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 <kad...@blackhole.kfki.hu>
> > ---
> > 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
> > <kad...@blackhole.kfki.hu>");
> > 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 : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
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
