* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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 0 siblings, 1 reply; 7+ 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] 7+ 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 0 siblings, 1 reply; 7+ 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] 7+ 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 0 siblings, 0 replies; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2026-04-07 8:03 UTC | newest]
Thread overview: 7+ 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-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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox