From 9802d21e7a0b0d2167ef745edc1f4ea7a0fc6ea3 Mon Sep 17 00:00:00 2001 From: Julian Anastasov Date: Thu, 12 Jun 2014 09:17:55 +0300 Subject: ipvs: stop tot_stats estimator only under CONFIG_SYSCTL The tot_stats estimator is started only when CONFIG_SYSCTL is defined. But it is stopped without checking CONFIG_SYSCTL. Fix the crash by moving ip_vs_stop_estimator into ip_vs_control_net_cleanup_sysctl. The change is needed after commit 14e405461e664b ("IPVS: Add __ip_vs_control_{init,cleanup}_sysctl()") from 2.6.39. Reported-by: Jet Chen Tested-by: Jet Chen Signed-off-by: Julian Anastasov Signed-off-by: Simon Horman --- net/netfilter/ipvs/ip_vs_ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index c42e83d2751..581a6584ed0 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -3778,6 +3778,7 @@ static void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net) cancel_delayed_work_sync(&ipvs->defense_work); cancel_work_sync(&ipvs->defense_work.work); unregister_net_sysctl_table(ipvs->sysctl_hdr); + ip_vs_stop_estimator(net, &ipvs->tot_stats); } #else @@ -3840,7 +3841,6 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net) struct netns_ipvs *ipvs = net_ipvs(net); ip_vs_trash_cleanup(net); - ip_vs_stop_estimator(net, &ipvs->tot_stats); ip_vs_control_net_cleanup_sysctl(net); remove_proc_entry("ip_vs_stats_percpu", net->proc_net); remove_proc_entry("ip_vs_stats", net->proc_net); -- cgit v1.2.3-70-g09d2 From b62b65055bcc5372d5c3f4103629176cb8db3678 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 5 Jun 2014 12:19:54 +0300 Subject: Bluetooth: Fix incorrectly overriding conn->src_type The src_type member of struct hci_conn should always reflect the address type of the src_member. It should never be overridden. There is already code in place in the command status handler of HCI_LE_Create_Connection to copy the right initiator address into conn->init_addr_type. Without this patch, if privacy is enabled, we will send the wrong address type in the SMP identity address information PDU (it'll e.g. contain our public address but a random address type). Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/hci_conn.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'net') diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 8671bc79a35..b9b2bd464be 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -610,11 +610,6 @@ static void hci_req_add_le_create_conn(struct hci_request *req, if (hci_update_random_address(req, false, &own_addr_type)) return; - /* Save the address type used for this connnection attempt so we able - * to retrieve this information if we need it. - */ - conn->src_type = own_addr_type; - cp.scan_interval = cpu_to_le16(hdev->le_scan_interval); cp.scan_window = cpu_to_le16(hdev->le_scan_window); bacpy(&cp.peer_addr, &conn->dst); -- cgit v1.2.3-70-g09d2 From e694788d73efe139b24f78b036deb97fe57fa8cb Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 10 Jun 2014 09:54:24 +0300 Subject: Bluetooth: Fix check for connection encryption The conn->link_key variable tracks the type of link key in use. It is set whenever we respond to a link key request as well as when we get a link key notification event. These two events do not however always guarantee that encryption is enabled: getting a link key request and responding to it may only mean that the remote side has requested authentication but not encryption. On the other hand, the encrypt change event is a certain guarantee that encryption is enabled. The real encryption state is already tracked in the conn->link_mode variable through the HCI_LM_ENCRYPT bit. This patch fixes a check for encryption in the hci_conn_auth function to use the proper conn->link_mode value and thereby eliminates the chance of a false positive result. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/hci_conn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index b9b2bd464be..ca01d186185 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -889,7 +889,7 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type) /* If we're already encrypted set the REAUTH_PEND flag, * otherwise set the ENCRYPT_PEND. */ - if (conn->key_type != 0xff) + if (conn->link_mode & HCI_LM_ENCRYPT) set_bit(HCI_CONN_REAUTH_PEND, &conn->flags); else set_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); -- cgit v1.2.3-70-g09d2 From ba15a58b179ed76a7e887177f2b06de12c58ec8f Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 9 Jun 2014 13:58:14 +0300 Subject: Bluetooth: Fix SSP acceptor just-works confirmation without MITM From the Bluetooth Core Specification 4.1 page 1958: "if both devices have set the Authentication_Requirements parameter to one of the MITM Protection Not Required options, authentication stage 1 shall function as if both devices set their IO capabilities to DisplayOnly (e.g., Numeric comparison with automatic confirmation on both devices)" So far our implementation has done user confirmation for all just-works cases regardless of the MITM requirements, however following the specification to the word means that we should not be doing confirmation when neither side has the MITM flag set. Signed-off-by: Johan Hedberg Tested-by: Szymon Janc Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/hci_event.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 21e5913d12e..ff11f4a1ada 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3628,8 +3628,11 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev, /* If we're not the initiators request authorization to * proceed from user space (mgmt_user_confirm with - * confirm_hint set to 1). */ - if (!test_bit(HCI_CONN_AUTH_PEND, &conn->flags)) { + * confirm_hint set to 1). The exception is if neither + * side had MITM in which case we do auto-accept. + */ + if (!test_bit(HCI_CONN_AUTH_PEND, &conn->flags) && + (loc_mitm || rem_mitm)) { BT_DBG("Confirming auto-accept as acceptor"); confirm_hint = 1; goto confirm; -- cgit v1.2.3-70-g09d2 From 4ad51a75c70ba1ba6802fa7ff2ee6829b1c6e61a Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 9 Jun 2014 14:41:25 +0300 Subject: Bluetooth: Add clarifying comment for conn->auth_type When responding to an IO capability request when we're the initiators of the pairing we will not yet have the remote IO capability information. Since the conn->auth_type variable is treated as an "absolute" requirement instead of a hint of what's needed later in the user confirmation request handler it's important that it doesn't have the MITM bit set if there's any chance that the remote device doesn't have the necessary IO capabilities. This patch adds a clarifying comment so that conn->auth_type is left untouched in this scenario. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/hci_event.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'net') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index ff11f4a1ada..3183edc2576 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3537,7 +3537,11 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb) cp.authentication = conn->auth_type; /* Request MITM protection if our IO caps allow it - * except for the no-bonding case + * except for the no-bonding case. + * conn->auth_type is not updated here since + * that might cause the user confirmation to be + * rejected in case the remote doesn't have the + * IO capabilities for MITM. */ if (conn->io_capability != HCI_IO_NO_INPUT_OUTPUT && cp.authentication != HCI_AT_NO_BONDING) -- cgit v1.2.3-70-g09d2 From fff3490f47810e2d34b91fb9e31103e923b11c2f Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 10 Jun 2014 15:19:50 +0300 Subject: Bluetooth: Fix setting correct authentication information for SMP STK When we store the STK in slave role we should set the correct authentication information for it. If the pairing is producing a HIGH security level the STK is considered authenticated, and otherwise it's considered unauthenticated. This patch fixes the value passed to the hci_add_ltk() function when adding the STK on the slave side. Signed-off-by: Johan Hedberg Tested-by: Marcin Kraglak Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/smp.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3d1cc164557..f2829a7932e 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -544,7 +544,7 @@ static u8 smp_random(struct smp_chan *smp) hci_le_start_enc(hcon, ediv, rand, stk); hcon->enc_key_size = smp->enc_key_size; } else { - u8 stk[16]; + u8 stk[16], auth; __le64 rand = 0; __le16 ediv = 0; @@ -556,8 +556,13 @@ static u8 smp_random(struct smp_chan *smp) memset(stk + smp->enc_key_size, 0, SMP_MAX_ENC_KEY_SIZE - smp->enc_key_size); + if (hcon->pending_sec_level == BT_SECURITY_HIGH) + auth = 1; + else + auth = 0; + hci_add_ltk(hcon->hdev, &hcon->dst, hcon->dst_type, - HCI_SMP_STK_SLAVE, 0, stk, smp->enc_key_size, + HCI_SMP_STK_SLAVE, auth, stk, smp->enc_key_size, ediv, rand); } -- cgit v1.2.3-70-g09d2 From 50143a433b70e3145bcf8a4a4e54f0c11bdee32b Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 10 Jun 2014 14:05:57 +0300 Subject: Bluetooth: Fix indicating discovery state when canceling inquiry When inquiry is canceled through the HCI_Cancel_Inquiry command there is no Inquiry Complete event generated. Instead, all we get is the command complete for the HCI_Inquiry_Cancel command. This means that we must call the hci_discovery_set_state() function from the respective command complete handler in order to ensure that user space knows the correct discovery state. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/hci_event.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'net') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 3183edc2576..640c54ec1bd 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -48,6 +48,10 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb) smp_mb__after_atomic(); /* wake_up_bit advises about this barrier */ wake_up_bit(&hdev->flags, HCI_INQUIRY); + hci_dev_lock(hdev); + hci_discovery_set_state(hdev, DISCOVERY_STOPPED); + hci_dev_unlock(hdev); + hci_conn_check_pending(hdev); } -- cgit v1.2.3-70-g09d2 From 21a60d307ddc2180cfa542a995d943d1034cf5c5 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 10 Jun 2014 14:05:58 +0300 Subject: Bluetooth: Refactor discovery stopping into its own function We'll need to reuse the same logic for stopping discovery also when cleaning up HCI state when powering off. This patch refactors the code out to its own function that can later (in a subsequent patch) be used also for the power off case. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/mgmt.c | 93 +++++++++++++++++++++++++++------------------------- 1 file changed, 49 insertions(+), 44 deletions(-) (limited to 'net') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 0fce54412ff..be6f0321912 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -1047,6 +1047,43 @@ static void clean_up_hci_complete(struct hci_dev *hdev, u8 status) } } +static void hci_stop_discovery(struct hci_request *req) +{ + struct hci_dev *hdev = req->hdev; + struct hci_cp_remote_name_req_cancel cp; + struct inquiry_entry *e; + + switch (hdev->discovery.state) { + case DISCOVERY_FINDING: + if (test_bit(HCI_INQUIRY, &hdev->flags)) { + hci_req_add(req, HCI_OP_INQUIRY_CANCEL, 0, NULL); + } else { + cancel_delayed_work(&hdev->le_scan_disable); + hci_req_add_le_scan_disable(req); + } + + break; + + case DISCOVERY_RESOLVING: + e = hci_inquiry_cache_lookup_resolve(hdev, BDADDR_ANY, + NAME_PENDING); + if (!e) + return; + + bacpy(&cp.bdaddr, &e->data.bdaddr); + hci_req_add(req, HCI_OP_REMOTE_NAME_REQ_CANCEL, sizeof(cp), + &cp); + + break; + + default: + /* Passive scanning */ + if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) + hci_req_add_le_scan_disable(req); + break; + } +} + static int clean_up_hci_state(struct hci_dev *hdev) { struct hci_request req; @@ -3574,8 +3611,6 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data, { struct mgmt_cp_stop_discovery *mgmt_cp = data; struct pending_cmd *cmd; - struct hci_cp_remote_name_req_cancel cp; - struct inquiry_entry *e; struct hci_request req; int err; @@ -3605,52 +3640,22 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data, hci_req_init(&req, hdev); - switch (hdev->discovery.state) { - case DISCOVERY_FINDING: - if (test_bit(HCI_INQUIRY, &hdev->flags)) { - hci_req_add(&req, HCI_OP_INQUIRY_CANCEL, 0, NULL); - } else { - cancel_delayed_work(&hdev->le_scan_disable); - - hci_req_add_le_scan_disable(&req); - } - - break; - - case DISCOVERY_RESOLVING: - e = hci_inquiry_cache_lookup_resolve(hdev, BDADDR_ANY, - NAME_PENDING); - if (!e) { - mgmt_pending_remove(cmd); - err = cmd_complete(sk, hdev->id, - MGMT_OP_STOP_DISCOVERY, 0, - &mgmt_cp->type, - sizeof(mgmt_cp->type)); - hci_discovery_set_state(hdev, DISCOVERY_STOPPED); - goto unlock; - } - - bacpy(&cp.bdaddr, &e->data.bdaddr); - hci_req_add(&req, HCI_OP_REMOTE_NAME_REQ_CANCEL, sizeof(cp), - &cp); + hci_stop_discovery(&req); - break; - - default: - BT_DBG("unknown discovery state %u", hdev->discovery.state); - - mgmt_pending_remove(cmd); - err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY, - MGMT_STATUS_FAILED, &mgmt_cp->type, - sizeof(mgmt_cp->type)); + err = hci_req_run(&req, stop_discovery_complete); + if (!err) { + hci_discovery_set_state(hdev, DISCOVERY_STOPPING); goto unlock; } - err = hci_req_run(&req, stop_discovery_complete); - if (err < 0) - mgmt_pending_remove(cmd); - else - hci_discovery_set_state(hdev, DISCOVERY_STOPPING); + mgmt_pending_remove(cmd); + + /* If no HCI commands were sent we're done */ + if (err == -ENODATA) { + err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY, 0, + &mgmt_cp->type, sizeof(mgmt_cp->type)); + hci_discovery_set_state(hdev, DISCOVERY_STOPPED); + } unlock: hci_dev_unlock(hdev); -- cgit v1.2.3-70-g09d2 From f8680f128b01212895a9afb31032f6ffe91bd771 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 10 Jun 2014 14:05:59 +0300 Subject: Bluetooth: Reuse hci_stop_discovery function when cleaning up HCI state When cleaning up the HCI state as part of the power-off procedure we can reuse the hci_stop_discovery() function instead of explicitly sending HCI command related to discovery. The added benefit of this is that it takes care of canceling name resolving and inquiry which were not previously covered by the code. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/mgmt.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'net') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index be6f0321912..6107e037cd8 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -1100,9 +1100,7 @@ static int clean_up_hci_state(struct hci_dev *hdev) if (test_bit(HCI_ADVERTISING, &hdev->dev_flags)) disable_advertising(&req); - if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) { - hci_req_add_le_scan_disable(&req); - } + hci_stop_discovery(&req); list_for_each_entry(conn, &hdev->conn_hash.list, list) { struct hci_cp_disconnect dc; -- cgit v1.2.3-70-g09d2 From 7ab56c3a6eccb215034b0cb096e0313441cbf2a4 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 12 Jun 2014 10:15:13 +0000 Subject: Bluetooth: Fix deadlock in l2cap_conn_del() A deadlock occurs when PDU containing invalid SMP opcode is received on Security Manager Channel over LE link and conn->pending_rx_work worker has not run yet. When LE link is created l2cap_conn_ready() is called and before returning it schedules conn->pending_rx_work worker to hdev->workqueue. Incoming data to SMP fixed channel is handled by l2cap_recv_frame() which calls smp_sig_channel() to handle the SMP PDU. If smp_sig_channel() indicates failure l2cap_conn_del() is called to delete the connection. When deleting the connection, l2cap_conn_del() purges the pending_rx queue and calls flush_work() to wait for the pending_rx_work worker to complete. Since incoming data is handled by a worker running from the same workqueue as the pending_rx_work is being scheduled on, we will deadlock on waiting for pending_rx_work to complete. This patch fixes the deadlock by calling cancel_work_sync() instead of flush_work(). Signed-off-by: Jukka Taimisto Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/l2cap_core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'net') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 6eabbe05fe5..323f23cd2c3 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1663,7 +1663,13 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err) kfree_skb(conn->rx_skb); skb_queue_purge(&conn->pending_rx); - flush_work(&conn->pending_rx_work); + + /* We can not call flush_work(&conn->pending_rx_work) here since we + * might block if we are running on a worker from the same workqueue + * pending_rx_work is waiting on. + */ + if (work_pending(&conn->pending_rx_work)) + cancel_work_sync(&conn->pending_rx_work); l2cap_unregister_all_users(conn); -- cgit v1.2.3-70-g09d2 From c73f94b8c093a615ce80eabbde0ac6eb9abfe31a Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Fri, 13 Jun 2014 10:22:28 +0300 Subject: Bluetooth: Fix locking of hdev when calling into SMP code The SMP code expects hdev to be unlocked since e.g. crypto functions will try to (re)lock it. Therefore, we need to release the lock before calling into smp.c from mgmt.c. Without this we risk a deadlock whenever the smp_user_confirm_reply() function is called. Signed-off-by: Johan Hedberg Tested-by: Lukasz Rymanowski Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/mgmt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'net') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 6107e037cd8..af8e0a6243b 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3031,8 +3031,13 @@ static int user_pairing_resp(struct sock *sk, struct hci_dev *hdev, } if (addr->type == BDADDR_LE_PUBLIC || addr->type == BDADDR_LE_RANDOM) { - /* Continue with pairing via SMP */ + /* Continue with pairing via SMP. The hdev lock must be + * released as SMP may try to recquire it for crypto + * purposes. + */ + hci_dev_unlock(hdev); err = smp_user_confirm_reply(conn, mgmt_op, passkey); + hci_dev_lock(hdev); if (!err) err = cmd_complete(sk, hdev->id, mgmt_op, -- cgit v1.2.3-70-g09d2 From 92d1372e1a9fec00e146b74e8b9ad7a385b9b37f Mon Sep 17 00:00:00 2001 From: Marcin Kraglak Date: Fri, 13 Jun 2014 14:08:22 +0200 Subject: Bluetooth: Allow change security level on ATT_CID in slave role Kernel supports SMP Security Request so don't block increasing security when we are slave. Signed-off-by: Marcin Kraglak Acked-by: Johan Hedberg Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org --- net/bluetooth/l2cap_sock.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'net') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index ade3fb4c23b..e1378693cc9 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -787,11 +787,6 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, /*change security for LE channels */ if (chan->scid == L2CAP_CID_ATT) { - if (!conn->hcon->out) { - err = -EINVAL; - break; - } - if (smp_conn_security(conn->hcon, sec.level)) break; sk->sk_state = BT_CONFIG; -- cgit v1.2.3-70-g09d2 From 266155b2de8fb721ae353688529b2f8bcdde2f90 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 5 Jun 2014 14:28:44 +0200 Subject: netfilter: ctnetlink: fix dumping of dying/unconfirmed conntracks The dumping prematurely stops, it seems the callback argument that indicates that all entries have been dumped is set after iterating on the first cpu list. The dumping also may stop before the entire per-cpu list content is also dumped. With this patch, conntrack -L dying now shows the dying list content again. Fixes: b7779d06 ("netfilter: conntrack: spinlock per cpu to protect special lists.") Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_netlink.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'net') diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 58579634427..ef0eedd7054 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1163,9 +1163,6 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying if (cb->args[2]) return 0; - if (cb->args[0] == nr_cpu_ids) - return 0; - for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) { struct ct_pcpu *pcpu; @@ -1194,6 +1191,7 @@ restart: rcu_read_unlock(); if (res < 0) { nf_conntrack_get(&ct->ct_general); + cb->args[0] = cpu; cb->args[1] = (unsigned long)ct; spin_unlock_bh(&pcpu->lock); goto out; @@ -1202,10 +1200,10 @@ restart: if (cb->args[1]) { cb->args[1] = 0; goto restart; - } else - cb->args[2] = 1; + } spin_unlock_bh(&pcpu->lock); } + cb->args[2] = 1; out: if (last) nf_ct_put(last); -- cgit v1.2.3-70-g09d2 From cd5f336f1780cb20e83146cde64d3d5779e175e6 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Sun, 8 Jun 2014 11:41:23 +0200 Subject: netfilter: ctnetlink: fix refcnt leak in dying/unconfirmed list dumper 'last' keeps track of the ct that had its refcnt bumped during previous dump cycle. Thus it must not be overwritten until end-of-function. Another (unrelated, theoretical) issue: Don't attempt to bump refcnt of a conntrack whose reference count is already 0. Such conntrack is being destroyed right now, its memory is freed once we release the percpu dying spinlock. Fixes: b7779d06 ('netfilter: conntrack: spinlock per cpu to protect special lists.') Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_netlink.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'net') diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index ef0eedd7054..70123f48b6c 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1150,7 +1150,7 @@ static int ctnetlink_done_list(struct netlink_callback *cb) static int ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying) { - struct nf_conn *ct, *last = NULL; + struct nf_conn *ct, *last; struct nf_conntrack_tuple_hash *h; struct hlist_nulls_node *n; struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh); @@ -1163,6 +1163,8 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying if (cb->args[2]) return 0; + last = (struct nf_conn *)cb->args[1]; + for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) { struct ct_pcpu *pcpu; @@ -1171,7 +1173,6 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu); spin_lock_bh(&pcpu->lock); - last = (struct nf_conn *)cb->args[1]; list = dying ? &pcpu->dying : &pcpu->unconfirmed; restart: hlist_nulls_for_each_entry(h, n, list, hnnode) { @@ -1190,7 +1191,8 @@ restart: ct); rcu_read_unlock(); if (res < 0) { - nf_conntrack_get(&ct->ct_general); + if (!atomic_inc_not_zero(&ct->ct_general.use)) + continue; cb->args[0] = cpu; cb->args[1] = (unsigned long)ct; spin_unlock_bh(&pcpu->lock); -- cgit v1.2.3-70-g09d2 From 5bc5c307653cbf8fe9da6cbd8ae6c6bd5b86ff4b Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 10 Jun 2014 10:53:00 +0200 Subject: netfilter: nf_tables: use RCU-safe list insertion when replacing rules The patch 5e94846 ("netfilter: nf_tables: add insert operation") did not include RCU-safe list insertion when replacing rules. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 624e083125b..ba37c10e513 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1796,7 +1796,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, goto err2; } nft_rule_disactivate_next(net, old_rule); - list_add_tail(&rule->list, &old_rule->list); + list_add_tail_rcu(&rule->list, &old_rule->list); } else { err = -ENOENT; goto err2; -- cgit v1.2.3-70-g09d2 From a0a7379e16b6e4c229d082f24c7e3ef9e812ed46 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 10 Jun 2014 10:53:01 +0200 Subject: netfilter: nf_tables: use u32 for chain use counter Since 4fefee5 ("netfilter: nf_tables: allow to delete several objects from a batch"), every new rule bumps the chain use counter. However, this is limited to 16 bits, which means that it will overrun after 2^16 rules. Use a u32 chain counter and check for overflows (just like we do for table objects). Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_tables.h | 6 +++--- net/netfilter/nf_tables_api.c | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'net') diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 7ee6ce6564a..713b0b88bd5 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -503,9 +503,9 @@ enum nft_chain_flags { * @net: net namespace that this chain belongs to * @table: table that this chain belongs to * @handle: chain handle - * @flags: bitmask of enum nft_chain_flags * @use: number of jump references to this chain * @level: length of longest path to this chain + * @flags: bitmask of enum nft_chain_flags * @name: name of the chain */ struct nft_chain { @@ -514,9 +514,9 @@ struct nft_chain { struct net *net; struct nft_table *table; u64 handle; - u8 flags; - u16 use; + u32 use; u16 level; + u8 flags; char name[NFT_CHAIN_MAXNAMELEN]; }; diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index ba37c10e513..5586426a616 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1730,6 +1730,9 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, if (!create || nlh->nlmsg_flags & NLM_F_REPLACE) return -EINVAL; handle = nf_tables_alloc_handle(table); + + if (chain->use == UINT_MAX) + return -EOVERFLOW; } if (nla[NFTA_RULE_POSITION]) { -- cgit v1.2.3-70-g09d2 From ac34b861979ec5057d686c890b1b8f8661e9b99f Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 10 Jun 2014 10:53:02 +0200 Subject: netfilter: nf_tables: decrement chain use counter when replacing rules Thus, the chain use counter remains with the same value after the rule replacement. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net') diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 5586426a616..19f438deeab 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1799,6 +1799,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, goto err2; } nft_rule_disactivate_next(net, old_rule); + chain->use--; list_add_tail_rcu(&rule->list, &old_rule->list); } else { err = -ENOENT; @@ -1829,6 +1830,7 @@ err3: list_del_rcu(&nft_trans_rule(trans)->list); nft_rule_clear(net, nft_trans_rule(trans)); nft_trans_destroy(trans); + chain->use++; } err2: nf_tables_rule_destroy(&ctx, rule); -- cgit v1.2.3-70-g09d2 From ac904ac835ac7879a9374dc3ef1e5cb75d9c7ceb Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 10 Jun 2014 10:53:03 +0200 Subject: netfilter: nf_tables: fix wrong type in transaction when replacing rules In b380e5c ("netfilter: nf_tables: add message type to transactions"), I used the wrong message type in the rule replacement case. The rule that is replaced needs to be handled as a deleted rule. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 19f438deeab..39369ea2df0 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1792,7 +1792,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, if (nlh->nlmsg_flags & NLM_F_REPLACE) { if (nft_rule_is_active_next(net, old_rule)) { - trans = nft_trans_rule_add(&ctx, NFT_MSG_NEWRULE, + trans = nft_trans_rule_add(&ctx, NFT_MSG_DELRULE, old_rule); if (trans == NULL) { err = -ENOMEM; -- cgit v1.2.3-70-g09d2 From 3d9b142131ef0cde259dbac5cce36f570fcb4902 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 11 Jun 2014 14:27:46 +0200 Subject: netfilter: nft_compat: call {target, match}->destroy() to cleanup entry Otherwise, the reference to external objects (eg. modules) are not released when the rules are removed. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nft_compat.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'net') diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c index 8a779be832f..1840989092e 100644 --- a/net/netfilter/nft_compat.c +++ b/net/netfilter/nft_compat.c @@ -195,6 +195,15 @@ static void nft_target_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { struct xt_target *target = expr->ops->data; + void *info = nft_expr_priv(expr); + struct xt_tgdtor_param par; + + par.net = ctx->net; + par.target = target; + par.targinfo = info; + par.family = ctx->afi->family; + if (par.target->destroy != NULL) + par.target->destroy(&par); module_put(target->me); } @@ -382,6 +391,15 @@ static void nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { struct xt_match *match = expr->ops->data; + void *info = nft_expr_priv(expr); + struct xt_mtdtor_param par; + + par.net = ctx->net; + par.match = match; + par.matchinfo = info; + par.family = ctx->afi->family; + if (par.match->destroy != NULL) + par.match->destroy(&par); module_put(match->me); } -- cgit v1.2.3-70-g09d2 From 6403d96254c7c44fdfa163248b1198c714c65f6a Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 11 Jun 2014 19:05:28 +0200 Subject: netfilter: nf_tables: indicate family when dumping set elements Set the nfnetlink header that indicates the family of this element. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 39369ea2df0..ab4566cfcbe 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2850,7 +2850,7 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) goto nla_put_failure; nfmsg = nlmsg_data(nlh); - nfmsg->nfgen_family = NFPROTO_UNSPEC; + nfmsg->nfgen_family = ctx.afi->family; nfmsg->version = NFNETLINK_V0; nfmsg->res_id = 0; -- cgit v1.2.3-70-g09d2 From 915136065b7ca75af4cae06281e4dc43926edbfe Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Fri, 13 Jun 2014 13:45:38 +0200 Subject: netfilter: nft_nat: don't dump port information if unset Don't include port information attributes if they are unset. Reported-by: Ana Rey Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nft_nat.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'net') diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c index a0195d28bcf..79ff58cd36d 100644 --- a/net/netfilter/nft_nat.c +++ b/net/netfilter/nft_nat.c @@ -175,12 +175,14 @@ static int nft_nat_dump(struct sk_buff *skb, const struct nft_expr *expr) if (nla_put_be32(skb, NFTA_NAT_REG_ADDR_MAX, htonl(priv->sreg_addr_max))) goto nla_put_failure; - if (nla_put_be32(skb, - NFTA_NAT_REG_PROTO_MIN, htonl(priv->sreg_proto_min))) - goto nla_put_failure; - if (nla_put_be32(skb, - NFTA_NAT_REG_PROTO_MAX, htonl(priv->sreg_proto_max))) - goto nla_put_failure; + if (priv->sreg_proto_min) { + if (nla_put_be32(skb, NFTA_NAT_REG_PROTO_MIN, + htonl(priv->sreg_proto_min))) + goto nla_put_failure; + if (nla_put_be32(skb, NFTA_NAT_REG_PROTO_MAX, + htonl(priv->sreg_proto_max))) + goto nla_put_failure; + } return 0; nla_put_failure: -- cgit v1.2.3-70-g09d2 From 4a001068d790366bbf64ee927a363f752abafa71 Mon Sep 17 00:00:00 2001 From: Ken-ichirou MATSUZAWA Date: Mon, 16 Jun 2014 13:52:34 +0200 Subject: netfilter: ctnetlink: add zone size to length Signed-off-by: Ken-ichirou MATSUZAWA Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_netlink.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'net') diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 70123f48b6c..300ed1eec72 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -596,6 +596,9 @@ ctnetlink_nlmsg_size(const struct nf_conn *ct) #endif #ifdef CONFIG_NF_CONNTRACK_MARK + nla_total_size(sizeof(u_int32_t)) /* CTA_MARK */ +#endif +#ifdef CONFIG_NF_CONNTRACK_ZONES + + nla_total_size(sizeof(u_int16_t)) /* CTA_ZONE */ #endif + ctnetlink_proto_size(ct) + ctnetlink_label_size(ct) @@ -2039,6 +2042,9 @@ ctnetlink_nfqueue_build_size(const struct nf_conn *ct) #endif #ifdef CONFIG_NF_CONNTRACK_MARK + nla_total_size(sizeof(u_int32_t)) /* CTA_MARK */ +#endif +#ifdef CONFIG_NF_CONNTRACK_ZONES + + nla_total_size(sizeof(u_int16_t)) /* CTA_ZONE */ #endif + ctnetlink_proto_size(ct) ; -- cgit v1.2.3-70-g09d2 From 945b2b2d259d1a4364a2799e80e8ff32f8c6ee6f Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Sat, 7 Jun 2014 21:17:04 +0200 Subject: netfilter: nf_nat: fix oops on netns removal Quoting Samu Kallio: Basically what's happening is, during netns cleanup, nf_nat_net_exit gets called before ipv4_net_exit. As I understand it, nf_nat_net_exit is supposed to kill any conntrack entries which have NAT context (through nf_ct_iterate_cleanup), but for some reason this doesn't happen (perhaps something else is still holding refs to those entries?). When ipv4_net_exit is called, conntrack entries (including those with NAT context) are cleaned up, but the nat_bysource hashtable is long gone - freed in nf_nat_net_exit. The bug happens when attempting to free a conntrack entry whose NAT hash 'prev' field points to a slot in the freed hash table (head for that bin). We ignore conntracks with null nat bindings. But this is wrong, as these are in bysource hash table as well. Restore nat-cleaning for the netns-is-being-removed case. bug: https://bugzilla.kernel.org/show_bug.cgi?id=65191 Fixes: c2d421e1718 ('netfilter: nf_nat: fix race when unloading protocol modules') Reported-by: Samu Kallio Debugged-by: Samu Kallio Signed-off-by: Florian Westphal Tested-by: Samu Kallio Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_nat_core.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'net') diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 09096a670c4..a49907b1dab 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -525,6 +525,39 @@ static int nf_nat_proto_remove(struct nf_conn *i, void *data) return i->status & IPS_NAT_MASK ? 1 : 0; } +static int nf_nat_proto_clean(struct nf_conn *ct, void *data) +{ + struct nf_conn_nat *nat = nfct_nat(ct); + + if (nf_nat_proto_remove(ct, data)) + return 1; + + if (!nat || !nat->ct) + return 0; + + /* This netns is being destroyed, and conntrack has nat null binding. + * Remove it from bysource hash, as the table will be freed soon. + * + * Else, when the conntrack is destoyed, nf_nat_cleanup_conntrack() + * will delete entry from already-freed table. + */ + if (!del_timer(&ct->timeout)) + return 1; + + spin_lock_bh(&nf_nat_lock); + hlist_del_rcu(&nat->bysource); + ct->status &= ~IPS_NAT_DONE_MASK; + nat->ct = NULL; + spin_unlock_bh(&nf_nat_lock); + + add_timer(&ct->timeout); + + /* don't delete conntrack. Although that would make things a lot + * simpler, we'd end up flushing all conntracks on nat rmmod. + */ + return 0; +} + static void nf_nat_l4proto_clean(u8 l3proto, u8 l4proto) { struct nf_nat_proto_clean clean = { @@ -795,7 +828,7 @@ static void __net_exit nf_nat_net_exit(struct net *net) { struct nf_nat_proto_clean clean = {}; - nf_ct_iterate_cleanup(net, &nf_nat_proto_remove, &clean, 0, 0); + nf_ct_iterate_cleanup(net, nf_nat_proto_clean, &clean, 0, 0); synchronize_rcu(); nf_ct_free_hashtable(net->ct.nat_bysource, net->ct.nat_htable_size); } -- cgit v1.2.3-70-g09d2 From 17846376f21c07c1b9ddfdef1a01bf3828fc1e06 Mon Sep 17 00:00:00 2001 From: Dave Jones Date: Mon, 16 Jun 2014 16:30:36 -0400 Subject: tcp: remove unnecessary tcp_sk assignment. This variable is overwritten by the child socket assignment before it ever gets used. Signed-off-by: Dave Jones Signed-off-by: David S. Miller --- net/ipv4/tcp_fastopen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c index 62e48cf84e6..9771563ab56 100644 --- a/net/ipv4/tcp_fastopen.c +++ b/net/ipv4/tcp_fastopen.c @@ -131,7 +131,7 @@ static bool tcp_fastopen_create_child(struct sock *sk, struct dst_entry *dst, struct request_sock *req) { - struct tcp_sock *tp = tcp_sk(sk); + struct tcp_sock *tp; struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue; struct sock *child; -- cgit v1.2.3-70-g09d2 From d36a4f4b472334562b8e7252e35d3d770db83815 Mon Sep 17 00:00:00 2001 From: Jie Liu Date: Tue, 17 Jun 2014 22:32:42 +0800 Subject: net: return actual error on register_queue_kobjects Return the actual error code if call kset_create_and_add() failed Cc: David S. Miller Signed-off-by: Jie Liu Signed-off-by: David S. Miller --- net/core/net-sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 1cac29ebb05..5c1c1e526a2 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1200,8 +1200,8 @@ static int register_queue_kobjects(struct net_device *net) #ifdef CONFIG_SYSFS net->queues_kset = kset_create_and_add("queues", NULL, &net->dev.kobj); - if (!net->queues_kset) - return -ENOMEM; + if (IS_ERR(net->queues_kset)) + return PTR_ERR(net->queues_kset); real_rx = net->real_num_rx_queues; #endif real_tx = net->real_num_tx_queues; -- cgit v1.2.3-70-g09d2 From ff5e92c1affe7166b3f6e7073e648ed65a6e2e59 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Thu, 19 Jun 2014 01:31:30 +0200 Subject: net: sctp: propagate sysctl errors from proc_do* properly sysctl handler proc_sctp_do_hmac_alg(), proc_sctp_do_rto_min() and proc_sctp_do_rto_max() do not properly reflect some error cases when writing values via sysctl from internal proc functions such as proc_dointvec() and proc_dostring(). In all these cases we pass the test for write != 0 and partially do additional work just to notice that additional sanity checks fail and we return with hard-coded -EINVAL while proc_do* functions might also return different errors. So fix this up by simply testing a successful return of proc_do* right after calling it. This also allows to propagate its return value onwards to the user. While touching this, also fix up some minor style issues. Fixes: 4f3fdf3bc59c ("sctp: add check rto_min and rto_max in sysctl") Fixes: 3c68198e7511 ("sctp: Make hmac algorithm selection for cookie generation dynamic") Signed-off-by: Daniel Borkmann Signed-off-by: David S. Miller --- net/sctp/sysctl.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-) (limited to 'net') diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c index dcb19592761..cc12162ba09 100644 --- a/net/sctp/sysctl.c +++ b/net/sctp/sysctl.c @@ -321,41 +321,40 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, int write, loff_t *ppos) { struct net *net = current->nsproxy->net_ns; - char tmp[8]; struct ctl_table tbl; - int ret; - int changed = 0; + bool changed = false; char *none = "none"; + char tmp[8]; + int ret; memset(&tbl, 0, sizeof(struct ctl_table)); if (write) { tbl.data = tmp; - tbl.maxlen = 8; + tbl.maxlen = sizeof(tmp); } else { tbl.data = net->sctp.sctp_hmac_alg ? : none; tbl.maxlen = strlen(tbl.data); } - ret = proc_dostring(&tbl, write, buffer, lenp, ppos); - if (write) { + ret = proc_dostring(&tbl, write, buffer, lenp, ppos); + if (write && ret == 0) { #ifdef CONFIG_CRYPTO_MD5 if (!strncmp(tmp, "md5", 3)) { net->sctp.sctp_hmac_alg = "md5"; - changed = 1; + changed = true; } #endif #ifdef CONFIG_CRYPTO_SHA1 if (!strncmp(tmp, "sha1", 4)) { net->sctp.sctp_hmac_alg = "sha1"; - changed = 1; + changed = true; } #endif if (!strncmp(tmp, "none", 4)) { net->sctp.sctp_hmac_alg = NULL; - changed = 1; + changed = true; } - if (!changed) ret = -EINVAL; } @@ -368,11 +367,10 @@ static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write, loff_t *ppos) { struct net *net = current->nsproxy->net_ns; - int new_value; - struct ctl_table tbl; unsigned int min = *(unsigned int *) ctl->extra1; unsigned int max = *(unsigned int *) ctl->extra2; - int ret; + struct ctl_table tbl; + int ret, new_value; memset(&tbl, 0, sizeof(struct ctl_table)); tbl.maxlen = sizeof(unsigned int); @@ -381,12 +379,15 @@ static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write, tbl.data = &new_value; else tbl.data = &net->sctp.rto_min; + ret = proc_dointvec(&tbl, write, buffer, lenp, ppos); - if (write) { - if (ret || new_value > max || new_value < min) + if (write && ret == 0) { + if (new_value > max || new_value < min) return -EINVAL; + net->sctp.rto_min = new_value; } + return ret; } @@ -395,11 +396,10 @@ static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write, loff_t *ppos) { struct net *net = current->nsproxy->net_ns; - int new_value; - struct ctl_table tbl; unsigned int min = *(unsigned int *) ctl->extra1; unsigned int max = *(unsigned int *) ctl->extra2; - int ret; + struct ctl_table tbl; + int ret, new_value; memset(&tbl, 0, sizeof(struct ctl_table)); tbl.maxlen = sizeof(unsigned int); @@ -408,12 +408,15 @@ static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write, tbl.data = &new_value; else tbl.data = &net->sctp.rto_max; + ret = proc_dointvec(&tbl, write, buffer, lenp, ppos); - if (write) { - if (ret || new_value > max || new_value < min) + if (write && ret == 0) { + if (new_value > max || new_value < min) return -EINVAL; + net->sctp.rto_max = new_value; } + return ret; } -- cgit v1.2.3-70-g09d2 From 6f9a093b66ce7cacc110d8737c03686e80ecfda6 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 18 Jun 2014 15:34:57 -0700 Subject: net: filter: fix upper BPF instruction limit The original checks (via sk_chk_filter) for instruction count uses ">", not ">=", so changing this in sk_convert_filter has the potential to break existing seccomp filters that used exactly BPF_MAXINSNS many instructions. Fixes: bd4cf0ed331a ("net: filter: rework/optimize internal BPF interpreter's instruction set") Signed-off-by: Kees Cook Cc: stable@vger.kernel.org # v3.15+ Acked-by: Daniel Borkmann Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/core/filter.c b/net/core/filter.c index 735fad89749..a44e12cdde4 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -840,7 +840,7 @@ int sk_convert_filter(struct sock_filter *prog, int len, BUILD_BUG_ON(BPF_MEMWORDS * sizeof(u32) > MAX_BPF_STACK); BUILD_BUG_ON(BPF_REG_FP + 1 != MAX_BPF_REG); - if (len <= 0 || len >= BPF_MAXINSNS) + if (len <= 0 || len > BPF_MAXINSNS) return -EINVAL; if (new_prog) { -- cgit v1.2.3-70-g09d2 From 8e4946ccdc09e0f04d2cb21f01886bd33de8532b Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Thu, 19 Jun 2014 18:12:15 -0700 Subject: Revert "net: return actual error on register_queue_kobjects" This reverts commit d36a4f4b472334562b8e7252e35d3d770db83815. Signed-off-by: David S. Miller --- net/core/net-sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 5c1c1e526a2..1cac29ebb05 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1200,8 +1200,8 @@ static int register_queue_kobjects(struct net_device *net) #ifdef CONFIG_SYSFS net->queues_kset = kset_create_and_add("queues", NULL, &net->dev.kobj); - if (IS_ERR(net->queues_kset)) - return PTR_ERR(net->queues_kset); + if (!net->queues_kset) + return -ENOMEM; real_rx = net->real_num_rx_queues; #endif real_tx = net->real_num_tx_queues; -- cgit v1.2.3-70-g09d2 From 2cd0d743b05e87445c54ca124a9916f22f16742e Mon Sep 17 00:00:00 2001 From: Neal Cardwell Date: Wed, 18 Jun 2014 21:15:03 -0400 Subject: tcp: fix tcp_match_skb_to_sack() for unaligned SACK at end of an skb If there is an MSS change (or misbehaving receiver) that causes a SACK to arrive that covers the end of an skb but is less than one MSS, then tcp_match_skb_to_sack() was rounding up pkt_len to the full length of the skb ("Round if necessary..."), then chopping all bytes off the skb and creating a zero-byte skb in the write queue. This was visible now because the recently simplified TLP logic in bef1909ee3ed1c ("tcp: fixing TLP's FIN recovery") could find that 0-byte skb at the end of the write queue, and now that we do not check that skb's length we could send it as a TLP probe. Consider the following example scenario: mss: 1000 skb: seq: 0 end_seq: 4000 len: 4000 SACK: start_seq: 3999 end_seq: 4000 The tcp_match_skb_to_sack() code will compute: in_sack = false pkt_len = start_seq - TCP_SKB_CB(skb)->seq = 3999 - 0 = 3999 new_len = (pkt_len / mss) * mss = (3999/1000)*1000 = 3000 new_len += mss = 4000 Previously we would find the new_len > skb->len check failing, so we would fall through and set pkt_len = new_len = 4000 and chop off pkt_len of 4000 from the 4000-byte skb, leaving a 0-byte segment afterward in the write queue. With this new commit, we notice that the new new_len >= skb->len check succeeds, so that we return without trying to fragment. Fixes: adb92db857ee ("tcp: Make SACK code to split only at mss boundaries") Reported-by: Eric Dumazet Signed-off-by: Neal Cardwell Cc: Eric Dumazet Cc: Yuchung Cheng Cc: Ilpo Jarvinen Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/ipv4/tcp_input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 40661fc1e23..b5c23756965 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1162,7 +1162,7 @@ static int tcp_match_skb_to_sack(struct sock *sk, struct sk_buff *skb, unsigned int new_len = (pkt_len / mss) * mss; if (!in_sack && new_len < pkt_len) { new_len += mss; - if (new_len > skb->len) + if (new_len >= skb->len) return 0; } pkt_len = new_len; -- cgit v1.2.3-70-g09d2 From 24599e61b7552673dd85971cf5a35369cd8c119e Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Wed, 18 Jun 2014 23:46:31 +0200 Subject: net: sctp: check proc_dointvec result in proc_sctp_do_auth When writing to the sysctl field net.sctp.auth_enable, it can well be that the user buffer we handed over to proc_dointvec() via proc_sctp_do_auth() handler contains something other than integers. In that case, we would set an uninitialized 4-byte value from the stack to net->sctp.auth_enable that can be leaked back when reading the sysctl variable, and it can unintentionally turn auth_enable on/off based on the stack content since auth_enable is interpreted as a boolean. Fix it up by making sure proc_dointvec() returned sucessfully. Fixes: b14878ccb7fa ("net: sctp: cache auth_enable per endpoint") Reported-by: Florian Westphal Signed-off-by: Daniel Borkmann Acked-by: Neil Horman Acked-by: Vlad Yasevich Signed-off-by: David S. Miller --- net/sctp/sysctl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'net') diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c index cc12162ba09..12c7e01c267 100644 --- a/net/sctp/sysctl.c +++ b/net/sctp/sysctl.c @@ -447,8 +447,7 @@ static int proc_sctp_do_auth(struct ctl_table *ctl, int write, tbl.data = &net->sctp.auth_enable; ret = proc_dointvec(&tbl, write, buffer, lenp, ppos); - - if (write) { + if (write && ret == 0) { struct sock *sk = net->sctp.ctl_sock; net->sctp.auth_enable = new_value; -- cgit v1.2.3-70-g09d2 From 916c1689a09bc1ca81f2d7a34876f8d35aadd11b Mon Sep 17 00:00:00 2001 From: Li RongQing Date: Wed, 18 Jun 2014 13:46:02 +0800 Subject: 8021q: fix a potential memory leak skb_cow called in vlan_reorder_header does not free the skb when it failed, and vlan_reorder_header returns NULL to reset original skb when it is called in vlan_untag, lead to a memory leak. Signed-off-by: Li RongQing Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/8021q/vlan_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'net') diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c index 9012b1c922b..75d42776399 100644 --- a/net/8021q/vlan_core.c +++ b/net/8021q/vlan_core.c @@ -114,8 +114,11 @@ EXPORT_SYMBOL(vlan_dev_vlan_proto); static struct sk_buff *vlan_reorder_header(struct sk_buff *skb) { - if (skb_cow(skb, skb_headroom(skb)) < 0) + if (skb_cow(skb, skb_headroom(skb)) < 0) { + kfree_skb(skb); return NULL; + } + memmove(skb->data - ETH_HLEN, skb->data - VLAN_ETH_HLEN, 2 * ETH_ALEN); skb->mac_header += VLAN_HLEN; return skb; -- cgit v1.2.3-70-g09d2 From 285276e72cbaa5be2147aac93133944882bced22 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 24 Jun 2014 15:33:20 +0200 Subject: trivial: net: filter: Fix typo in comment Signed-off-by: Tobias Klauser Acked-by: Daniel Borkmann Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/core/filter.c b/net/core/filter.c index a44e12cdde4..ff9235e10b1 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1382,7 +1382,7 @@ static struct sk_filter *__sk_migrate_realloc(struct sk_filter *fp, fp_new = sock_kmalloc(sk, len, GFP_KERNEL); if (fp_new) { *fp_new = *fp; - /* As we're kepping orig_prog in fp_new along, + /* As we're keeping orig_prog in fp_new along, * we need to make sure we're not evicting it * from the old fp. */ -- cgit v1.2.3-70-g09d2 From 677a9fd3e6e6e03e11b979b69c9f8c813583683a Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 24 Jun 2014 15:33:21 +0200 Subject: trivial: net: filter: Change kerneldoc parameter order Change the order of the parameters to sk_unattached_filter_create() in the kerneldoc to reflect the order they appear in the actual function. This fix is only cosmetic, in the generated doc they still appear in the correct order without the fix. Signed-off-by: Tobias Klauser Acked-by: Daniel Borkmann Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/core/filter.c b/net/core/filter.c index ff9235e10b1..4d13b125ef4 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1524,8 +1524,8 @@ static struct sk_filter *__sk_prepare_filter(struct sk_filter *fp, /** * sk_unattached_filter_create - create an unattached filter - * @fprog: the filter program * @pfp: the unattached filter that is created + * @fprog: the filter program * * Create a filter independent of any socket. We first run some * sanity checks on it to make sure it does not explode on us later. -- cgit v1.2.3-70-g09d2 From 99e72a0fed07d118d329f3046ad2ec2ae9357d63 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 24 Jun 2014 15:33:22 +0200 Subject: net: filter: Use kcalloc/kmalloc_array to allocate arrays Use kcalloc/kmalloc_array to make it clear we're allocating arrays. No integer overflow can actually happen here, since len/flen is guaranteed to be less than BPF_MAXINSNS (4096). However, this changed makes sure we're not going to get one if BPF_MAXINSNS were ever increased. Signed-off-by: Tobias Klauser Acked-by: Daniel Borkmann Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/core/filter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/core/filter.c b/net/core/filter.c index 4d13b125ef4..1dbf6462f76 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -844,7 +844,7 @@ int sk_convert_filter(struct sock_filter *prog, int len, return -EINVAL; if (new_prog) { - addrs = kzalloc(len * sizeof(*addrs), GFP_KERNEL); + addrs = kcalloc(len, sizeof(*addrs), GFP_KERNEL); if (!addrs) return -ENOMEM; } @@ -1101,7 +1101,7 @@ static int check_load_and_stores(struct sock_filter *filter, int flen) BUILD_BUG_ON(BPF_MEMWORDS > 16); - masks = kmalloc(flen * sizeof(*masks), GFP_KERNEL); + masks = kmalloc_array(flen, sizeof(*masks), GFP_KERNEL); if (!masks) return -ENOMEM; -- cgit v1.2.3-70-g09d2 From f88649721268999bdff09777847080a52004f691 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Tue, 24 Jun 2014 10:05:11 -0700 Subject: ipv4: fix dst race in sk_dst_get() When IP route cache had been removed in linux-3.6, we broke assumption that dst entries were all freed after rcu grace period. DST_NOCACHE dst were supposed to be freed from dst_release(). But it appears we want to keep such dst around, either in UDP sockets or tunnels. In sk_dst_get() we need to make sure dst refcount is not 0 before incrementing it, or else we might end up freeing a dst twice. DST_NOCACHE set on a dst does not mean this dst can not be attached to a socket or a tunnel. Then, before actual freeing, we need to observe a rcu grace period to make sure all other cpus can catch the fact the dst is no longer usable. Signed-off-by: Eric Dumazet Reported-by: Dormando Signed-off-by: David S. Miller --- include/net/sock.h | 4 ++-- net/core/dst.c | 16 +++++++++++----- net/ipv4/ip_tunnel.c | 14 +++++--------- 3 files changed, 18 insertions(+), 16 deletions(-) (limited to 'net') diff --git a/include/net/sock.h b/include/net/sock.h index 07b7fcd60d8..173cae485de 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1730,8 +1730,8 @@ sk_dst_get(struct sock *sk) rcu_read_lock(); dst = rcu_dereference(sk->sk_dst_cache); - if (dst) - dst_hold(dst); + if (dst && !atomic_inc_not_zero(&dst->__refcnt)) + dst = NULL; rcu_read_unlock(); return dst; } diff --git a/net/core/dst.c b/net/core/dst.c index 80d6286c8b6..a028409ee43 100644 --- a/net/core/dst.c +++ b/net/core/dst.c @@ -269,6 +269,15 @@ again: } EXPORT_SYMBOL(dst_destroy); +static void dst_destroy_rcu(struct rcu_head *head) +{ + struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head); + + dst = dst_destroy(dst); + if (dst) + __dst_free(dst); +} + void dst_release(struct dst_entry *dst) { if (dst) { @@ -276,11 +285,8 @@ void dst_release(struct dst_entry *dst) newrefcnt = atomic_dec_return(&dst->__refcnt); WARN_ON(newrefcnt < 0); - if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) { - dst = dst_destroy(dst); - if (dst) - __dst_free(dst); - } + if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) + call_rcu(&dst->rcu_head, dst_destroy_rcu); } } EXPORT_SYMBOL(dst_release); diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 097b3e7c1e8..54b6731dab5 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -73,12 +73,7 @@ static void __tunnel_dst_set(struct ip_tunnel_dst *idst, { struct dst_entry *old_dst; - if (dst) { - if (dst->flags & DST_NOCACHE) - dst = NULL; - else - dst_clone(dst); - } + dst_clone(dst); old_dst = xchg((__force struct dst_entry **)&idst->dst, dst); dst_release(old_dst); } @@ -108,13 +103,14 @@ static struct rtable *tunnel_rtable_get(struct ip_tunnel *t, u32 cookie) rcu_read_lock(); dst = rcu_dereference(this_cpu_ptr(t->dst_cache)->dst); + if (dst && !atomic_inc_not_zero(&dst->__refcnt)) + dst = NULL; if (dst) { if (dst->obsolete && dst->ops->check(dst, cookie) == NULL) { - rcu_read_unlock(); tunnel_dst_reset(t); - return NULL; + dst_release(dst); + dst = NULL; } - dst_hold(dst); } rcu_read_unlock(); return (struct rtable *)dst; -- cgit v1.2.3-70-g09d2 From de843723f9b989178762196fb24dd050cbe20ca3 Mon Sep 17 00:00:00 2001 From: Tom Herbert Date: Wed, 25 Jun 2014 12:51:01 -0700 Subject: net: fix setting csum_start in skb_segment() Dave Jones reported that a crash is occurring in csum_partial tcp_gso_segment inet_gso_segment ? update_dl_migration skb_mac_gso_segment __skb_gso_segment dev_hard_start_xmit sch_direct_xmit __dev_queue_xmit ? dev_hard_start_xmit dev_queue_xmit ip_finish_output ? ip_output ip_output ip_forward_finish ip_forward ip_rcv_finish ip_rcv __netif_receive_skb_core ? __netif_receive_skb_core ? trace_hardirqs_on __netif_receive_skb netif_receive_skb_internal napi_gro_complete ? napi_gro_complete dev_gro_receive ? dev_gro_receive napi_gro_receive It looks like a likely culprit is that SKB_GSO_CB()->csum_start is not set correctly when doing non-scatter gather. We are using offset as opposed to doffset. Reported-by: Dave Jones Tested-by: Dave Jones Signed-off-by: Tom Herbert Signed-off-by: Eric Dumazet Fixes: 7e2b10c1e52ca ("net: Support for multiple checksums with gso") Acked-by: Tom Herbert Signed-off-by: David S. Miller --- net/core/skbuff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 9cd5344fad7..c1a33033cbe 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2993,7 +2993,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, skb_put(nskb, len), len, 0); SKB_GSO_CB(nskb)->csum_start = - skb_headroom(nskb) + offset; + skb_headroom(nskb) + doffset; continue; } -- cgit v1.2.3-70-g09d2