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

[subashab@codeaurora.org]

On 2020-11-22 12:35, Florian Westphal wrote:
> Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> 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.