All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Oester <kernel@linuxace.com>
To: Michal Kubecek <mkubecek@suse.cz>
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Patrick McHardy <kaber@trash.net>,
	Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Subject: Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
Date: Wed, 11 Sep 2013 10:09:47 -0700	[thread overview]
Message-ID: <20130911170946.GA2926@linuxace.com> (raw)
In-Reply-To: <20130911152804.GA5397@unicorn.suse.cz>

On Wed, Sep 11, 2013 at 05:28:05PM +0200, Michal Kubecek wrote:
> > Looking at
> > your proposed fix, the NAT extension data should have been cleaned
> > from the bysource list in nf_nat_cleanup_conntrack (via __nf_ct_ext_destroy)
> > before reaching the kfree.  Would you agree?
> 
> It is cleaned from the list but as it is an RCU list, other readers can
> still be holding pointers to it. We have to wait for the RCU grace
> period before we can reuse it.

Agreed - looks like your fix should work.  However, two nits:

1) normally RCU functions have _rcu suffixes.  So nf_ct_ext_free should
become nf_ct_ext_free_rcu.

2) kfree_rcu was not added to the kernel until 3.0.  All of the bug
reports I've been looking into (including the original in netfilter bugzilla
at http://bugzilla.netfilter.org/show_bug.cgi?id=714) have been reported in
2.6.32 or earlier kernels.  So a different fix would need to be backported for
-stable.  For that, we would probably export __nf_ct_ext_free_rcu from
nf_conntrack_extend.c and change the kfree call in nf_ct_ext_free_rcu to
call_rcu(&ct->ext->rcu, __nf_ct_ext_free_rcu). Of course the alternative
is just to use this fix for both old and new kernels for simplicity.

> No, it is a bugreport from our customer. And even that customer
> encountered it only once so far. Which is not very surprising as to
> reproduce it, you have to be (un)lucky twice: first to have someone
> overwrite the area soon enough and second to have someone access the
> area after it is overwritten.

Yes, hitting this seems dependent upon phase of the moon.

Phil

  reply	other threads:[~2013-09-11 17:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-11  8:17 [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions Michal Kubecek
2013-09-11 14:57 ` Phil Oester
2013-09-11 15:28   ` Michal Kubecek
2013-09-11 17:09     ` Phil Oester [this message]
2013-09-11 17:42       ` Michal Kubecek
2013-09-11 17:54         ` Eric Dumazet
2013-09-11 17:50       ` Pablo Neira Ayuso

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=20130911170946.GA2926@linuxace.com \
    --to=kernel@linuxace.com \
    --cc=kaber@trash.net \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.