Re: [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu

[Haoze Xie <royenheart@gmail.com>]

On 4/6/2026 6:33 PM, Ido Schimmel wrote:
> On Sat, Apr 04, 2026 at 07:52:03PM +0800, Ao Zhou wrote:
>> From: Haoze Xie <royenheart@gmail.com>
>>
>> l3mdev_fib_table_rcu() assumes that any upper device observed for
>> an IFF_L3MDEV_SLAVE device is an L3 master and dereferences
>> master->l3mdev_ops unconditionally.
>>
>> VRF slave setup sets IFF_L3MDEV_SLAVE before the upper link is fully
>> switched, so readers can transiently observe a non-L3 upper such as a
>> bridge and follow a NULL l3mdev_ops pointer. Require the current upper
>> to still be an L3 master before consulting its FIB table.
> 
> Do you have a reproducer? I don't see how that can happen.
> 
> do_set_master() ensures that the device doesn't have a master when
> ndo_add_slave() is called. Meaning, if netif_is_l3_slave() is true and
> netdev_master_upper_dev_get_rcu() resolved a master device, it's
> guaranteed to be a VRF.
> 

I agree that this is correct in the synchronous `do_set_master()`
control flow.

My concern is narrower: an RCU reader can still resolve a stale upper
after the unlink, so `netif_is_l3_slave(dev)` alone is not enough to
prove that the resolved upper is still an L3 master at the point where
`master->l3mdev_ops` is dereferenced.

>>
>> Fixes: fee6d4c777a1 ("net: Add netif_is_l3_slave")
>> Reported-by: Yifan Wu <yifanwucs@gmail.com>
>> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
>> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
>> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
>> Suggested-by: Xin Liu <bird@lzu.edu.cn>
>> Signed-off-by: Haoze Xie <royenheart@gmail.com>
>> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
>> ---
>>  net/l3mdev/l3mdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
>> index 5432a5f2dfc8..b8a3030cb2c4 100644
>> --- a/net/l3mdev/l3mdev.c
>> +++ b/net/l3mdev/l3mdev.c
>> @@ -177,7 +177,7 @@ u32 l3mdev_fib_table_rcu(const struct net_device *dev)
>>  		const struct net_device *master;
>>
>>  		master = netdev_master_upper_dev_get_rcu(_dev);
>> -		if (master &&
>> +		if (master && netif_is_l3_master(master) &&
>>  		    master->l3mdev_ops->l3mdev_fib_table)
>>  			tb_id = master->l3mdev_ops->l3mdev_fib_table(master);
>>  	}
>> --
>> 2.53.0
>>