All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Oester <kernel@linuxace.com>
Cc: Michal Kubecek <mkubecek@suse.cz>,
	netfilter-devel@vger.kernel.org, netdev@vger.kernel.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 19:50:43 +0200	[thread overview]
Message-ID: <20130911175043.GA4824@localhost> (raw)
In-Reply-To: <20130911170946.GA2926@linuxace.com>

On Wed, Sep 11, 2013 at 10:09:47AM -0700, Phil Oester wrote:
> 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.

That postfix is there if the function requires to be called holding
rcu read lock, not this case. I'll take this patch.

> 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.

Either way, we need a specific backport for 2.6.x indeed.

Thanks for tracking up this issue.

      parent reply	other threads:[~2013-09-11 17:50 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
2013-09-11 17:42       ` Michal Kubecek
2013-09-11 17:54         ` Eric Dumazet
2013-09-11 17:50       ` Pablo Neira Ayuso [this message]

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=20130911175043.GA4824@localhost \
    --to=pablo@netfilter.org \
    --cc=kaber@trash.net \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=kernel@linuxace.com \
    --cc=mkubecek@suse.cz \
    --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 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.