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

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

[Ao Zhou]

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: 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

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

[David Ahern]

On 4/4/26 5:52 AM, 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: fee6d4c777a1 ("net: Add netif_is_l3_slave")

Fixes should be:

commit fdeea7be88b12742bfd50d9e19a06c0d2e702400
Author: Ido Schimmel <idosch@mellanox.com>
Date:   Thu Mar 16 09:08:15 2017 +0100

    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>
> 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) &&

Reviewed-by: David Ahern <dsahern@kernel.org>

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

[Ido Schimmel]

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.

> 
> 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
> 

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

[Ao Zhou]

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)
 			tb_id = master->l3mdev_ops->l3mdev_fib_table(master);
 	}
-- 
2.53.0

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

[Ido Schimmel]

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
> 

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

[David Ahern]

On 4/6/26 9:48 AM, Ido Schimmel wrote:
> 
> 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).

synchronize_rcu after the unlink (control path) seems like a better,
more robust option than adding more checks to the datapath.

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

[Ido Schimmel]

On Mon, Apr 06, 2026 at 12:14:15PM -0600, David Ahern wrote:
> On 4/6/26 9:48 AM, Ido Schimmel wrote:
> > 
> > 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).
> 
> synchronize_rcu after the unlink (control path) seems like a better,
> more robust option than adding more checks to the datapath.

IMO it would be better to proceed with the original approach and look
into adding RCU synchronization in net-next. The original approach is
more surgical and the pattern of first checking netif_is_l3_master()
already exists in other places.