Re: [PATCH 5/5] netfilter: ipset: Fix calling ip_set() macro at dumping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pablo,

On Mon, 29 Oct 2018, Pablo Neira Ayuso wrote:

> On Sat, Oct 27, 2018 at 06:05:43PM +0200, Jozsef Kadlecsik wrote:
> > The ip_set() macro is called when either ip_set_ref_lock held only
> > or no lock/nfnl mutex is held at dumping. Take this into account
> > properly.
> > 
> > Signed-off-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>
> > ---
> >  net/netfilter/ipset/ip_set_core.c | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> > index 68db946..292f7c7 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -55,12 +55,27 @@ MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>");
> >  MODULE_DESCRIPTION("core IP set support");
> >  MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
> >  
> > -/* When the nfnl mutex is held: */
> > +/* When the nfnl mutex or ip_set_ref_lock is held: */
> >  #define ip_set_dereference(p)		\
> > -	rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
> > +	rcu_dereference_protected(p,	\
> > +		lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET) || \
> > +		lockdep_is_held(&ip_set_ref_lock))
> >  #define ip_set(inst, id)		\
> >  	ip_set_dereference((inst)->ip_set_list)[id]
> >  
> > +/* When the nfnl mutex is not held (dumping): */
> > +static inline
> > +struct ip_set * ip_set_no_mutex(struct ip_set_net *inst, ip_set_id_t id)
> > +{
> > +	struct ip_set *set;
> > +
> > +	rcu_read_lock();
> > +	set = rcu_dereference((inst)->ip_set_list)[id];
> > +	rcu_read_unlock();
> 
> If your goal is to calm down bogus warnings, you can use instead:
> 
>         set = rcu_dereference_raw(...);
> 
> instead, in case ip_set_dump_done() is already protected under
> rcu_read_lock(). Otherwise, you have to rework your rcu approach in
> the dump_done path.

Yes, you are completely right: the goal of the patch was to silence bogus
warnings but that part is useless, as you pointed out.

> rcu is not useful in the way you use it above, because future
> dereference of set are only valid insider the rcu read lock section.
> 
> Would you send me a follow up patch to amend this?

I'm going to send you a fixed patch.

Best regards,
Jozsef
-
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux