| From 5195c14c8b27cc0b18220ddbf0e5ad3328a04187 Mon Sep 17 00:00:00 2001 |
| From: bill bonaparte <programme110@gmail.com> |
| Date: Thu, 6 Nov 2014 14:36:48 +0100 |
| Subject: netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse |
| |
| From: bill bonaparte <programme110@gmail.com> |
| |
| commit 5195c14c8b27cc0b18220ddbf0e5ad3328a04187 upstream. |
| |
| After removal of the central spinlock nf_conntrack_lock, in |
| commit 93bb0ceb75be2 ("netfilter: conntrack: remove central |
| spinlock nf_conntrack_lock"), it is possible to race against |
| get_next_corpse(). |
| |
| The race is against the get_next_corpse() cleanup on |
| the "unconfirmed" list (a per-cpu list with seperate locking), |
| which set the DYING bit. |
| |
| Fix this race, in __nf_conntrack_confirm(), by removing the CT |
| from unconfirmed list before checking the DYING bit. In case |
| race occured, re-add the CT to the dying list. |
| |
| While at this, fix coding style of the comment that has been |
| updated. |
| |
| Fixes: 93bb0ceb75be2 ("netfilter: conntrack: remove central spinlock nf_conntrack_lock") |
| Reported-by: bill bonaparte <programme110@gmail.com> |
| Signed-off-by: bill bonaparte <programme110@gmail.com> |
| Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> |
| Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/netfilter/nf_conntrack_core.c | 14 ++++++++------ |
| 1 file changed, 8 insertions(+), 6 deletions(-) |
| |
| --- a/net/netfilter/nf_conntrack_core.c |
| +++ b/net/netfilter/nf_conntrack_core.c |
| @@ -611,12 +611,16 @@ __nf_conntrack_confirm(struct sk_buff *s |
| */ |
| NF_CT_ASSERT(!nf_ct_is_confirmed(ct)); |
| pr_debug("Confirming conntrack %p\n", 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 */ |
| + |
| + /* 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); |
| |
| 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; |
| @@ -636,8 +640,6 @@ __nf_conntrack_confirm(struct sk_buff *s |
| 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. */ |
