public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
       [not found] <cover.1775062214.git.royenheart@gmail.com>
@ 2026-04-04 11:52 ` Ao Zhou
  2026-04-05 16:22   ` David Ahern
  2026-04-06 10:33   ` Ido Schimmel
  2026-04-06 13:28 ` [PATCH net v2 " Ao Zhou
  1 sibling, 2 replies; 11+ messages in thread
From: Ao Zhou @ 2026-04-04 11:52 UTC (permalink / raw)
  To: netdev
  Cc: David Ahern, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu,
	Ao Zhou, Haoze Xie

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-04 11:52 ` [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu Ao Zhou
@ 2026-04-05 16:22   ` David Ahern
  2026-04-06 10:33   ` Ido Schimmel
  1 sibling, 0 replies; 11+ messages in thread
From: David Ahern @ 2026-04-05 16:22 UTC (permalink / raw)
  To: Ao Zhou, netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu, Haoze Xie

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>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-04 11:52 ` [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu Ao Zhou
  2026-04-05 16:22   ` David Ahern
@ 2026-04-06 10:33   ` Ido Schimmel
  2026-04-19  3:45     ` Haoze Xie
  1 sibling, 1 reply; 11+ messages in thread
From: Ido Schimmel @ 2026-04-06 10:33 UTC (permalink / raw)
  To: Ao Zhou
  Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Yifan Wu, Juefei Pu,
	Yuan Tan, Xin Liu, Haoze Xie

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
       [not found] <cover.1775062214.git.royenheart@gmail.com>
  2026-04-04 11:52 ` [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu Ao Zhou
@ 2026-04-06 13:28 ` Ao Zhou
  2026-04-06 15:48   ` Ido Schimmel
  1 sibling, 1 reply; 11+ messages in thread
From: Ao Zhou @ 2026-04-06 13:28 UTC (permalink / raw)
  To: netdev, David Ahern
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Ido Schimmel, Jiri Pirko, Yifan Wu, Juefei Pu,
	Yuan Tan, Xin Liu, Ao Zhou, Haoze Xie

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-06 13:28 ` [PATCH net v2 " Ao Zhou
@ 2026-04-06 15:48   ` Ido Schimmel
  2026-04-06 18:14     ` David Ahern
  2026-04-19  3:49     ` Haoze Xie
  0 siblings, 2 replies; 11+ messages in thread
From: Ido Schimmel @ 2026-04-06 15:48 UTC (permalink / raw)
  To: Ao Zhou
  Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Ido Schimmel,
	Jiri Pirko, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu, Haoze Xie

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-06 15:48   ` Ido Schimmel
@ 2026-04-06 18:14     ` David Ahern
  2026-04-07  8:02       ` Ido Schimmel
  2026-04-19  3:49     ` Haoze Xie
  1 sibling, 1 reply; 11+ messages in thread
From: David Ahern @ 2026-04-06 18:14 UTC (permalink / raw)
  To: Ido Schimmel, Ao Zhou
  Cc: netdev, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Ido Schimmel, Jiri Pirko, Yifan Wu,
	Juefei Pu, Yuan Tan, Xin Liu, Haoze Xie

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-06 18:14     ` David Ahern
@ 2026-04-07  8:02       ` Ido Schimmel
  2026-04-19  3:51         ` Haoze Xie
  0 siblings, 1 reply; 11+ messages in thread
From: Ido Schimmel @ 2026-04-07  8:02 UTC (permalink / raw)
  To: David Ahern
  Cc: Ao Zhou, netdev, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Ido Schimmel, Jiri Pirko, Yifan Wu,
	Juefei Pu, Yuan Tan, Xin Liu, Haoze Xie

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-06 10:33   ` Ido Schimmel
@ 2026-04-19  3:45     ` Haoze Xie
  0 siblings, 0 replies; 11+ messages in thread
From: Haoze Xie @ 2026-04-19  3:45 UTC (permalink / raw)
  To: Ido Schimmel, Ao Zhou
  Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Yifan Wu, Juefei Pu,
	Yuan Tan, Xin Liu


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-06 15:48   ` Ido Schimmel
  2026-04-06 18:14     ` David Ahern
@ 2026-04-19  3:49     ` Haoze Xie
  2026-04-19 14:38       ` Haoze Xie
  1 sibling, 1 reply; 11+ messages in thread
From: Haoze Xie @ 2026-04-19  3:49 UTC (permalink / raw)
  To: Ido Schimmel, Ao Zhou
  Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Ido Schimmel,
	Jiri Pirko, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu, royenheart


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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-07  8:02       ` Ido Schimmel
@ 2026-04-19  3:51         ` Haoze Xie
  0 siblings, 0 replies; 11+ messages in thread
From: Haoze Xie @ 2026-04-19  3:51 UTC (permalink / raw)
  To: Ido Schimmel, David Ahern
  Cc: Ao Zhou, netdev, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Ido Schimmel, Jiri Pirko, Yifan Wu,
	Juefei Pu, Yuan Tan, Xin Liu, royenheart


On 4/7/2026 4:02 PM, Ido Schimmel wrote:
> 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.

I followed that direction for v3.

The patch keeps the more surgical datapath fix for net and extends the
same check to the other same-pattern slave-side helpers.

I am sending v3 as a separate public thread. Further feedback is
welcome.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
  2026-04-19  3:49     ` Haoze Xie
@ 2026-04-19 14:38       ` Haoze Xie
  0 siblings, 0 replies; 11+ messages in thread
From: Haoze Xie @ 2026-04-19 14:38 UTC (permalink / raw)
  To: Ido Schimmel, Ao Zhou
  Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Ido Schimmel,
	Jiri Pirko, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu, royenheart


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-04-19 14:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1775062214.git.royenheart@gmail.com>
2026-04-04 11:52 ` [PATCH net 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu Ao Zhou
2026-04-05 16:22   ` David Ahern
2026-04-06 10:33   ` Ido Schimmel
2026-04-19  3:45     ` Haoze Xie
2026-04-06 13:28 ` [PATCH net v2 " Ao Zhou
2026-04-06 15:48   ` Ido Schimmel
2026-04-06 18:14     ` David Ahern
2026-04-07  8:02       ` Ido Schimmel
2026-04-19  3:51         ` Haoze Xie
2026-04-19  3:49     ` Haoze Xie
2026-04-19 14:38       ` Haoze Xie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox