Re: [PATCH nf] netfilter: x_tables: Switch synchronization to RCU

[subashab@xxxxxxxxxxxxxx]

On 2020-11-22 12:35, Florian Westphal wrote:
> Subash Abhinov Kasiviswanathan <subashab@xxxxxxxxxxxxxx> wrote:
> > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> > index af22dbe..416a617 100644
> > --- a/net/netfilter/x_tables.c
> > +++ b/net/netfilter/x_tables.c
> > @@ -1349,6 +1349,14 @@ struct xt_counters *xt_counters_alloc(unsigned 
> > int counters)
> >  }
> >  EXPORT_SYMBOL(xt_counters_alloc);
> [..]
>
> >  	/* Do the substitution. */
> > -	local_bh_disable();
> > -	private = table->private;
> > +	private = xt_table_get_private_protected(table);
> >
> >  	/* Check inside lock: is the old number correct? */
> >  	if (num_counters != private->number) {
>
> There is a local_bh_enable() here that needs removal.

Thanks, will update that.

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 416a617..acce622 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1379,7 +1379,6 @@ xt_replace_table(struct xt_table *table,
        if (num_counters != private->number) {
                pr_debug("num_counters != table->private->number 
(%u/%u)\n",
                         num_counters, private->number);
-               local_bh_enable();
                *error = -EAGAIN;
                return NULL;
        }

> Did you test it with PROVE_LOCKING enabled?
>
> The placement/use of rcu_dereference and the _protected version
> looks correct, I would not expect splats.

My config doesn't seem to have it. I will enable and try it out.