From b9e11747e1227d7ad67c5b80be4b206e4059687e Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Wed, 11 Jan 2012 09:26:54 +0100 Subject: mac80211: fix rx->key NULL pointer dereference in promiscuous mode commit 1140afa862842ac3e56678693050760edc4ecde9 upstream. Since: commit 816c04fe7ef01dd9649f5ccfe796474db8708be5 Author: Christian Lamparter Date: Sat Apr 30 15:24:30 2011 +0200 mac80211: consolidate MIC failure report handling is possible to that we dereference rx->key == NULL when driver set RX_FLAG_MMIC_STRIPPED and not RX_FLAG_IV_STRIPPED and we are in promiscuous mode. This happen with rt73usb and rt61pci at least. Before the commit we always check rx->key against NULL, so I assume fix should be done in mac80211 (also mic_fail path has similar check). References: https://bugzilla.redhat.com/show_bug.cgi?id=769766 http://rt2x00.serialmonkey.com/pipermail/users_rt2x00.serialmonkey.com/2012-January/004395.html Reported-by: Stuart D Gathman Reported-by: Kai Wohlfahrt Signed-off-by: Stanislaw Gruszka Signed-off-by: John W. Linville Signed-off-by: Greg Kroah-Hartman --- net/mac80211/wpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c index 8f6a302d2ac3..aa1c40ab6a7c 100644 --- a/net/mac80211/wpa.c +++ b/net/mac80211/wpa.c @@ -109,7 +109,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx) if (status->flag & RX_FLAG_MMIC_ERROR) goto mic_fail; - if (!(status->flag & RX_FLAG_IV_STRIPPED)) + if (!(status->flag & RX_FLAG_IV_STRIPPED) && rx->key) goto update_iv; return RX_CONTINUE; -- cgit v1.2.3 From b09577ca6680033a4315e2f5cb3a95ebbb8dea79 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Thu, 22 Dec 2011 18:22:49 -0700 Subject: svcrpc: fix double-free on shutdown of nfsd after changing pool mode commit 61c8504c428edcebf23b97775a129c5b393a302b upstream. The pool_to and to_pool fields of the global svc_pool_map are freed on shutdown, but are initialized in nfsd startup only in the SVC_POOL_PERCPU and SVC_POOL_PERNODE cases. They *are* initialized to zero on kernel startup. So as long as you use only SVC_POOL_GLOBAL (the default), this will never be a problem. You're also OK if you only ever use SVC_POOL_PERCPU or SVC_POOL_PERNODE. However, the following sequence events leads to a double-free: 1. set SVC_POOL_PERCPU or SVC_POOL_PERNODE 2. start nfsd: both fields are initialized. 3. shutdown nfsd: both fields are freed. 4. set SVC_POOL_GLOBAL 5. start nfsd: the fields are left untouched. 6. shutdown nfsd: now we try to free them again. Step 4 is actually unnecessary, since (for some bizarre reason), nfsd automatically resets the pool mode to SVC_POOL_GLOBAL on shutdown. Signed-off-by: J. Bruce Fields Signed-off-by: Greg Kroah-Hartman --- net/sunrpc/svc.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net') diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 2b90292e9505..131da58efdc8 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -167,6 +167,7 @@ svc_pool_map_alloc_arrays(struct svc_pool_map *m, unsigned int maxpools) fail_free: kfree(m->to_pool); + m->to_pool = NULL; fail: return -ENOMEM; } @@ -287,7 +288,9 @@ svc_pool_map_put(void) if (!--m->count) { m->mode = SVC_POOL_DEFAULT; kfree(m->to_pool); + m->to_pool = NULL; kfree(m->pool_to); + m->pool_to = NULL; m->npools = 0; } -- cgit v1.2.3 From 7df22768c0af8769d805f6db21144d71d91fe13d Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Tue, 29 Nov 2011 11:35:35 -0500 Subject: svcrpc: destroy server sockets all at once commit 2fefb8a09e7ed251ae8996e0c69066e74c5aa560 upstream. There's no reason I can see that we need to call sv_shutdown between closing the two lists of sockets. Signed-off-by: J. Bruce Fields Signed-off-by: Greg Kroah-Hartman --- net/sunrpc/svc.c | 7 +------ net/sunrpc/svc_xprt.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 7 deletions(-) (limited to 'net') diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 131da58efdc8..4d5cb99194c4 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -476,16 +476,11 @@ svc_destroy(struct svc_serv *serv) del_timer_sync(&serv->sv_temptimer); - svc_close_all(&serv->sv_tempsocks); + svc_close_all(serv); if (serv->sv_shutdown) serv->sv_shutdown(serv); - svc_close_all(&serv->sv_permsocks); - - BUG_ON(!list_empty(&serv->sv_permsocks)); - BUG_ON(!list_empty(&serv->sv_tempsocks)); - cache_clean_deferred(serv); if (svc_serv_is_pooled(serv)) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index bd31208bbb61..9cb2621a8826 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -936,7 +936,7 @@ void svc_close_xprt(struct svc_xprt *xprt) } EXPORT_SYMBOL_GPL(svc_close_xprt); -void svc_close_all(struct list_head *xprt_list) +static void svc_close_list(struct list_head *xprt_list) { struct svc_xprt *xprt; struct svc_xprt *tmp; @@ -954,6 +954,15 @@ void svc_close_all(struct list_head *xprt_list) } } +void svc_close_all(struct svc_serv *serv) +{ + svc_close_list(&serv->sv_tempsocks); + svc_close_list(&serv->sv_permsocks); + BUG_ON(!list_empty(&serv->sv_permsocks)); + BUG_ON(!list_empty(&serv->sv_tempsocks)); + +} + /* * Handle defer and revisit of requests */ -- cgit v1.2.3 From a141a5eb3ab45131cb168e7a561d662722b43ec3 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Tue, 29 Nov 2011 17:00:26 -0500 Subject: svcrpc: avoid memory-corruption on pool shutdown commit b4f36f88b3ee7cf26bf0be84e6c7fc15f84dcb71 upstream. Socket callbacks use svc_xprt_enqueue() to add an xprt to a pool->sp_sockets list. In normal operation a server thread will later come along and take the xprt off that list. On shutdown, after all the threads have exited, we instead manually walk the sv_tempsocks and sv_permsocks lists to find all the xprt's and delete them. So the sp_sockets lists don't really matter any more. As a result, we've mostly just ignored them and hoped they would go away. Which has gotten us into trouble; witness for example ebc63e531cc6 "svcrpc: fix list-corrupting race on nfsd shutdown", the result of Ben Greear noticing that a still-running svc_xprt_enqueue() could re-add an xprt to an sp_sockets list just before it was deleted. The fix was to remove it from the list at the end of svc_delete_xprt(). But that only made corruption less likely--I can see nothing that prevents a svc_xprt_enqueue() from adding another xprt to the list at the same moment that we're removing this xprt from the list. In fact, despite the earlier xpo_detach(), I don't even see what guarantees that svc_xprt_enqueue() couldn't still be running on this xprt. So, instead, note that svc_xprt_enqueue() essentially does: lock sp_lock if XPT_BUSY unset add to sp_sockets unlock sp_lock So, if we do: set XPT_BUSY on every xprt. Empty every sp_sockets list, under the sp_socks locks. Then we're left knowing that the sp_sockets lists are all empty and will stay that way, since any svc_xprt_enqueue() will check XPT_BUSY under the sp_lock and see it set. And *then* we can continue deleting the xprt's. (Thanks to Jeff Layton for being correctly suspicious of this code....) Cc: Ben Greear Cc: Jeff Layton Signed-off-by: J. Bruce Fields Signed-off-by: Greg Kroah-Hartman --- net/sunrpc/svc.c | 10 +++++++++- net/sunrpc/svc_xprt.c | 48 +++++++++++++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 20 deletions(-) (limited to 'net') diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 4d5cb99194c4..ce5f111fe325 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -475,7 +475,15 @@ svc_destroy(struct svc_serv *serv) printk("svc_destroy: no threads for serv=%p!\n", serv); del_timer_sync(&serv->sv_temptimer); - + /* + * The set of xprts (contained in the sv_tempsocks and + * sv_permsocks lists) is now constant, since it is modified + * only by accepting new sockets (done by service threads in + * svc_recv) or aging old ones (done by sv_temptimer), or + * configuration changes (excluded by whatever locking the + * caller is using--nfsd_mutex in the case of nfsd). So it's + * safe to traverse those lists and shut everything down: + */ svc_close_all(serv); if (serv->sv_shutdown) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 9cb2621a8826..9d7ed0b48b51 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -901,14 +901,7 @@ void svc_delete_xprt(struct svc_xprt *xprt) spin_lock_bh(&serv->sv_lock); if (!test_and_set_bit(XPT_DETACHED, &xprt->xpt_flags)) list_del_init(&xprt->xpt_list); - /* - * The only time we're called while xpt_ready is still on a list - * is while the list itself is about to be destroyed (in - * svc_destroy). BUT svc_xprt_enqueue could still be attempting - * to add new entries to the sp_sockets list, so we can't leave - * a freed xprt on it. - */ - list_del_init(&xprt->xpt_ready); + BUG_ON(!list_empty(&xprt->xpt_ready)); if (test_bit(XPT_TEMP, &xprt->xpt_flags)) serv->sv_tmpcnt--; spin_unlock_bh(&serv->sv_lock); @@ -939,28 +932,45 @@ EXPORT_SYMBOL_GPL(svc_close_xprt); static void svc_close_list(struct list_head *xprt_list) { struct svc_xprt *xprt; - struct svc_xprt *tmp; - /* - * The server is shutting down, and no more threads are running. - * svc_xprt_enqueue() might still be running, but at worst it - * will re-add the xprt to sp_sockets, which will soon get - * freed. So we don't bother with any more locking, and don't - * leave the close to the (nonexistent) server threads: - */ - list_for_each_entry_safe(xprt, tmp, xprt_list, xpt_list) { + list_for_each_entry(xprt, xprt_list, xpt_list) { set_bit(XPT_CLOSE, &xprt->xpt_flags); - svc_delete_xprt(xprt); + set_bit(XPT_BUSY, &xprt->xpt_flags); } } void svc_close_all(struct svc_serv *serv) { + struct svc_pool *pool; + struct svc_xprt *xprt; + struct svc_xprt *tmp; + int i; + svc_close_list(&serv->sv_tempsocks); svc_close_list(&serv->sv_permsocks); + + for (i = 0; i < serv->sv_nrpools; i++) { + pool = &serv->sv_pools[i]; + + spin_lock_bh(&pool->sp_lock); + while (!list_empty(&pool->sp_sockets)) { + xprt = list_first_entry(&pool->sp_sockets, struct svc_xprt, xpt_ready); + list_del_init(&xprt->xpt_ready); + } + spin_unlock_bh(&pool->sp_lock); + } + /* + * At this point the sp_sockets lists will stay empty, since + * svc_enqueue will not add new entries without taking the + * sp_lock and checking XPT_BUSY. + */ + list_for_each_entry_safe(xprt, tmp, &serv->sv_tempsocks, xpt_list) + svc_delete_xprt(xprt); + list_for_each_entry_safe(xprt, tmp, &serv->sv_permsocks, xpt_list) + svc_delete_xprt(xprt); + BUG_ON(!list_empty(&serv->sv_permsocks)); BUG_ON(!list_empty(&serv->sv_tempsocks)); - } /* -- cgit v1.2.3 From d253520a7b2c2223fb4f704f06d10f2c547bdeef Mon Sep 17 00:00:00 2001 From: Nick Bowler Date: Tue, 8 Nov 2011 12:12:44 +0000 Subject: ah: Correctly pass error codes in ahash output callback. commit 069294e813ed5f27f82613b027609bcda5f1b914 upstream. The AH4/6 ahash output callbacks pass nexthdr to xfrm_output_resume instead of the error code. This appears to be a copy+paste error from the input case, where nexthdr is expected. This causes the driver to continuously add AH headers to the datagram until either an allocation fails and the packet is dropped or the ahash driver hits a synchronous fallback and the resulting monstrosity is transmitted. Correct this issue by simply passing the error code unadulterated. Signed-off-by: Nick Bowler Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/ipv4/ah4.c | 2 -- net/ipv6/ah6.c | 2 -- 2 files changed, 4 deletions(-) (limited to 'net') diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c index c1f4154552fc..33ca18603e3b 100644 --- a/net/ipv4/ah4.c +++ b/net/ipv4/ah4.c @@ -136,8 +136,6 @@ static void ah_output_done(struct crypto_async_request *base, int err) memcpy(top_iph+1, iph+1, top_iph->ihl*4 - sizeof(struct iphdr)); } - err = ah->nexthdr; - kfree(AH_SKB_CB(skb)->tmp); xfrm_output_resume(skb, err); } diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c index 2195ae651923..ede4d9d6cc2b 100644 --- a/net/ipv6/ah6.c +++ b/net/ipv6/ah6.c @@ -324,8 +324,6 @@ static void ah6_output_done(struct crypto_async_request *base, int err) #endif } - err = ah->nexthdr; - kfree(AH_SKB_CB(skb)->tmp); xfrm_output_resume(skb, err); } -- cgit v1.2.3 From c0ab420c6822529fa5aba05668e1e983b065460f Mon Sep 17 00:00:00 2001 From: Nick Bowler Date: Tue, 8 Nov 2011 12:12:45 +0000 Subject: ah: Read nexthdr value before overwriting it in ahash input callback. commit b7ea81a58adc123a4e980cb0eff9eb5c144b5dc7 upstream. The AH4/6 ahash input callbacks read out the nexthdr field from the AH header *after* they overwrite that header. This is obviously not going to end well. Fix it up. Signed-off-by: Nick Bowler Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/ipv4/ah4.c | 4 ++-- net/ipv6/ah6.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'net') diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c index 33ca18603e3b..c7056b2e831f 100644 --- a/net/ipv4/ah4.c +++ b/net/ipv4/ah4.c @@ -262,12 +262,12 @@ static void ah_input_done(struct crypto_async_request *base, int err) if (err) goto out; + err = ah->nexthdr; + skb->network_header += ah_hlen; memcpy(skb_network_header(skb), work_iph, ihl); __skb_pull(skb, ah_hlen + ihl); skb_set_transport_header(skb, -ihl); - - err = ah->nexthdr; out: kfree(AH_SKB_CB(skb)->tmp); xfrm_input_resume(skb, err); diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c index ede4d9d6cc2b..7a33aaa00227 100644 --- a/net/ipv6/ah6.c +++ b/net/ipv6/ah6.c @@ -464,12 +464,12 @@ static void ah6_input_done(struct crypto_async_request *base, int err) if (err) goto out; + err = ah->nexthdr; + skb->network_header += ah_hlen; memcpy(skb_network_header(skb), work_iph, hdr_len); __skb_pull(skb, ah_hlen + hdr_len); skb_set_transport_header(skb, -hdr_len); - - err = ah->nexthdr; out: kfree(AH_SKB_CB(skb)->tmp); xfrm_input_resume(skb, err); -- cgit v1.2.3