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