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

[Ido Schimmel <idosch@nvidia.com>]

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.

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
>