Re: [PATCH nft] netfilter: nf_tables: Fix pertential data-race in __nft_flowtable_type_get()

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

 



> On Tue, Apr 02, 2024 at 12:56:42PM +0200, Florian Westphal wrote:
>> Ziyang Xuan <william.xuanziyang@xxxxxxxxxx> wrote:
>>> nft_unregister_flowtable_type() within nf_flow_inet_module_exit() can
>>> concurrent with __nft_flowtable_type_get() within nf_tables_newflowtable().
>>> And thhere is not any protection when iterate over nf_tables_flowtables
>>> list in __nft_flowtable_type_get(). Therefore, there is pertential
>>> data-race of nf_tables_flowtables list entry.
>>>
>>> Use list_for_each_entry_rcu() with rcu_read_lock() to Iterate over
>>> nf_tables_flowtables list in __nft_flowtable_type_get() to resolve it.
>>
>> I don't think this resolves the described race.
>>
>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@xxxxxxxxxx>
>>> ---
>>>  net/netfilter/nf_tables_api.c | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>>> index fd86f2720c9e..fbf38e32f11d 100644
>>> --- a/net/netfilter/nf_tables_api.c
>>> +++ b/net/netfilter/nf_tables_api.c
>>> @@ -8297,10 +8297,14 @@ static const struct nf_flowtable_type *__nft_flowtable_type_get(u8 family)
>>>  {
>>>  	const struct nf_flowtable_type *type;
>>>  
>>> -	list_for_each_entry(type, &nf_tables_flowtables, list) {
>>> -		if (family == type->family)
>>> +	rcu_read_lock()
>>> +	list_for_each_entry_rcu(type, &nf_tables_flowtables, list) {
>>> +		if (family == type->family) {
>>> +			rcu_read_unlock();
>>>  			return type;
>>
>> This means 'type' can be non-null while module is being unloaded,
>> before refcount increment.
>>
>> You need acquire rcu_read_lock() in the caller, nft_flowtable_type_get,
>> and release it after refcount on module owner failed or succeeded.
> 
> And these need to be rcu protected:
> 
> static LIST_HEAD(nf_tables_expressions);
> static LIST_HEAD(nf_tables_objects);
> static LIST_HEAD(nf_tables_flowtables);
> 
> for a complete fix for:
> 
> f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")
> .
> 

I would be happy to do these. Thank you for your kind remind!


William Xuan
Best regards.








[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux