From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org, brouer@redhat.com
Subject: [PATCH net] Revert "netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse"
Date: Tue, 25 Nov 2014 19:54:47 +0100 [thread overview]
Message-ID: <1416941687-25471-1-git-send-email-pablo@netfilter.org> (raw)
This reverts commit 5195c14c8b27cc0b18220ddbf0e5ad3328a04187.
If the conntrack clashes with an existing one, it is left out of
the unconfirmed list, thus, crashing when dropping the packet and
releasing the conntrack since golden rule is that conntracks are
always placed in any of the existing lists for traceability reasons.
Reported-by: Daniel Borkmann <dborkman@redhat.com>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=88841
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
Hi David,
Could you manually apply this to your net tree? We have a better
candidate fix to replace this broken patch that I will pass to you
once it gets sufficient testing.
Thanks!
net/netfilter/nf_conntrack_core.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 2c69975..5016a69 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -611,16 +611,12 @@ __nf_conntrack_confirm(struct sk_buff *skb)
*/
NF_CT_ASSERT(!nf_ct_is_confirmed(ct));
pr_debug("Confirming conntrack %p\n", ct);
-
- /* We have to check the DYING flag after unlink to prevent
- * a race against nf_ct_get_next_corpse() possibly called from
- * user context, else we insert an already 'dead' hash, blocking
- * further use of that particular connection -JM.
- */
- nf_ct_del_from_dying_or_unconfirmed_list(ct);
+ /* We have to check the DYING flag inside the lock to prevent
+ a race against nf_ct_get_next_corpse() possibly called from
+ user context, else we insert an already 'dead' hash, blocking
+ further use of that particular connection -JM */
if (unlikely(nf_ct_is_dying(ct))) {
- nf_ct_add_to_dying_list(ct);
nf_conntrack_double_unlock(hash, reply_hash);
local_bh_enable();
return NF_ACCEPT;
@@ -640,6 +636,8 @@ __nf_conntrack_confirm(struct sk_buff *skb)
zone == nf_ct_zone(nf_ct_tuplehash_to_ctrack(h)))
goto out;
+ nf_ct_del_from_dying_or_unconfirmed_list(ct);
+
/* Timer relative to confirmation time, not original
setting time, otherwise we'd get timer wrap in
weird delay cases. */
--
1.7.10.4
next reply other threads:[~2014-11-25 18:54 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-25 18:54 Pablo Neira Ayuso [this message]
2014-11-25 20:00 ` [PATCH net] Revert "netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse" David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1416941687-25471-1-git-send-email-pablo@netfilter.org \
--to=pablo@netfilter.org \
--cc=brouer@redhat.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).