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

[Haoze Xie <royenheart@gmail.com>]

On 4/19/2026 11:49 AM, Haoze Xie wrote:
> 
> On 4/6/2026 11:48 PM, Ido Schimmel wrote:
>> On Mon, Apr 06, 2026 at 09:28:16PM +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.
>>>
>>> Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
>>> 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>
>>> Reviewed-by: David Ahern <dsahern@kernel.org>
>>> Signed-off-by: Haoze Xie <royenheart@gmail.com>
>>> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
>>> ---
>>> changes in v2:
>>> - point Fixes to the VRF slave ordering change identified by David Ahern
>>> - add David Ahern's Reviewed-by trailer
>>>
>>>  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)
>>
>> Don't we have the same problem in l3mdev_l3_rcv() and l3mdev_l3_out()?
>> If so, please check if I missed more places and include them in v3.
>>
> 
> I checked the same pattern in the other slave-side helpers, and v3 now
> extends the fix to both `l3mdev_l3_rcv()` and `l3mdev_l3_out()` in
> addition to `l3mdev_fib_table_rcu()`.
> 
> All three helpers resolve the current upper with
> `netdev_master_upper_dev_get_rcu()` and then use `master->l3mdev_ops`.
> So v3 consistently requires the resolved upper to still satisfy
> `netif_is_l3_master(master)` before dereferencing `l3mdev_ops`.
> 

While updating the patch, I found that `l3mdev_l3_rcv()` must keep
working for existing `IFF_L3MDEV_RX_HANDLER` users such as
`ipvlan_l3s`. The v3 patch keeps that direct RX-handler path
intact and applies the extra master check only to the slave-resolved
upper case.

the smoke test, it should ping succeffully:

===> BEGIN smoke test cmd <===
ip netns add ipvl_ns
ip link add ipvl_host type veth peer name ipvl_peer
ip link set ipvl_peer netns ipvl_ns

ip link set ipvl_host up
ip link add link ipvl_host name ipvl0 type ipvlan mode l3s
ip addr add 198.51.100.1/24 dev ipvl0
ip link set ipvl0 up

ip netns exec ipvl_ns ip link set lo up
ip netns exec ipvl_ns ip link set ipvl_peer up
ip netns exec ipvl_ns ip addr add 198.51.100.2/24 dev ipvl_peer
ip netns exec ipvl_ns ping -c 3 198.51.100.1
===> END smoke test cmd <===

>> And I think that the part that I was missing earlier is that we don't
>> have RCU synchronization in the unslaving path, so an RCU reader can
>> either see the original master, NULL or a new master (e.g., bridge
>> instead of the original VRF master).
>>
>>>  			tb_id = master->l3mdev_ops->l3mdev_fib_table(master);
>>>  	}
>>> -- 
>>> 2.53.0
>>>
>