[PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[Ren Wei]

From: Haoze Xie <royenheart@gmail.com>

Several l3mdev slave-side helpers resolve an upper device and then use
l3mdev_ops without first proving that the resolved device is still a
valid L3 master.

During slave transition, an RCU reader can transiently observe an upper
that is not an L3 master. Guard the affected slave-resolved paths by
requiring the resolved upper to still be an L3 master before using
l3mdev_ops, while keeping existing L3 RX handler providers intact.

Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
Cc: stable@kernel.org
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>
Tested-by: Haoze Xie <royenheart@gmail.com>
Signed-off-by: Haoze Xie <royenheart@gmail.com>
Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
---
Changes in v3:
- Extend the same guard to l3mdev_l3_rcv() and l3mdev_l3_out().
- Keep existing IFF_L3MDEV_RX_HANDLER providers such as ipvlan_l3s
  working by only applying the extra master check to the slave-resolved
  upper case in l3mdev_l3_rcv().
- v2 Link:
  https://lore.kernel.org/all/429dd4a81d4ca5624ab9f6d7b53c5fe08552c734.1775443332.git.royenheart@gmail.com/

Changes in v2:
- Point Fixes to the VRF slave ordering change identified in review.
- Add David Ahern's Reviewed-by trailer in that revision.
- v1 Link:
  https://lore.kernel.org/all/b3b88cddc7e79d4b43756b26ae5db965678f3ba9.1775062214.git.royenheart@gmail.com/

 include/net/l3mdev.h | 18 +++++++++++-------
 net/l3mdev/l3mdev.c  |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/net/l3mdev.h b/include/net/l3mdev.h
index 710e98665eb3..aed52bf03956 100644
--- a/include/net/l3mdev.h
+++ b/include/net/l3mdev.h
@@ -180,14 +180,17 @@ struct sk_buff *l3mdev_l3_rcv(struct sk_buff *skb, u16 proto)
 {
 	struct net_device *master = NULL;
 
-	if (netif_is_l3_slave(skb->dev))
+	if (netif_is_l3_slave(skb->dev)) {
 		master = netdev_master_upper_dev_get_rcu(skb->dev);
-	else if (netif_is_l3_master(skb->dev) ||
-		 netif_has_l3_rx_handler(skb->dev))
+		if (master && netif_is_l3_master(master) &&
+		    master->l3mdev_ops->l3mdev_l3_rcv)
+			skb = master->l3mdev_ops->l3mdev_l3_rcv(master, skb, proto);
+	} else if (netif_is_l3_master(skb->dev) ||
+		   netif_has_l3_rx_handler(skb->dev)) {
 		master = skb->dev;
-
-	if (master && master->l3mdev_ops->l3mdev_l3_rcv)
-		skb = master->l3mdev_ops->l3mdev_l3_rcv(master, skb, proto);
+		if (master->l3mdev_ops->l3mdev_l3_rcv)
+			skb = master->l3mdev_ops->l3mdev_l3_rcv(master, skb, proto);
+	}
 
 	return skb;
 }
@@ -215,7 +218,8 @@ struct sk_buff *l3mdev_l3_out(struct sock *sk, struct sk_buff *skb, u16 proto)
 		struct net_device *master;
 
 		master = netdev_master_upper_dev_get_rcu(dev);
-		if (master && master->l3mdev_ops->l3mdev_l3_out)
+		if (master && netif_is_l3_master(master) &&
+		    master->l3mdev_ops->l3mdev_l3_out)
 			skb = master->l3mdev_ops->l3mdev_l3_out(master, sk,
 								skb, proto);
 	}
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 v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[Ido Schimmel]

On Sun, Apr 19, 2026 at 10:53:32PM +0800, Ren Wei wrote:
> From: Haoze Xie <royenheart@gmail.com>
> 
> Several l3mdev slave-side helpers resolve an upper device and then use
> l3mdev_ops without first proving that the resolved device is still a
> valid L3 master.
> 
> During slave transition, an RCU reader can transiently observe an upper
> that is not an L3 master. Guard the affected slave-resolved paths by
> requiring the resolved upper to still be an L3 master before using
> l3mdev_ops, while keeping existing L3 RX handler providers intact.
> 
> Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
> Cc: stable@kernel.org
> 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>
> Tested-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>

I think it's fine for net:

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

We can try a more general approach in net-next which ensures that RCU
readers don't see different master devices within an RCU read-side
critical section. Something like [1]. I tested it (without your patch)
using the reproducer and was not able to trigger the bug.

I am aware that there are more places where we can unlink a device from
its master (e.g., deleting a master device), but most / all of them
(needs auditing) result in a synchronize_rcu() before releasing RTNL.

On my machine, the time it takes to unlink 1k devices from their master
increased from 0.96 seconds to 1.22 seconds (on average), which is
probably fine.

[1]
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 69daba3ddaf0..23657a11c66d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2970,6 +2970,7 @@ static int do_set_master(struct net_device *dev, int ifindex,
                        netdev_lock_ops(dev);
                        if (err)
                                return err;
+                       synchronize_net();
                } else {
                        return -EOPNOTSUPP;
                }

Re: [PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[Ido Schimmel]

On Mon, Apr 20, 2026 at 02:32:08PM +0300, Ido Schimmel wrote:
> On Sun, Apr 19, 2026 at 10:53:32PM +0800, Ren Wei wrote:
> > From: Haoze Xie <royenheart@gmail.com>
> > 
> > Several l3mdev slave-side helpers resolve an upper device and then use
> > l3mdev_ops without first proving that the resolved device is still a
> > valid L3 master.
> > 
> > During slave transition, an RCU reader can transiently observe an upper
> > that is not an L3 master. Guard the affected slave-resolved paths by
> > requiring the resolved upper to still be an L3 master before using
> > l3mdev_ops, while keeping existing L3 RX handler providers intact.
> > 
> > Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
> > Cc: stable@kernel.org
> > 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>
> > Tested-by: Haoze Xie <royenheart@gmail.com>
> > Signed-off-by: Haoze Xie <royenheart@gmail.com>
> > Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
> 
> I think it's fine for net:
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Thought about this again. I would like to check another approach
(synchronize_net() after clearing IFF_L3MDEV_SLAVE). Will update
tomorrow.

Re: [PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[Ido Schimmel]

On Mon, Apr 20, 2026 at 09:26:50PM +0300, Ido Schimmel wrote:
> On Mon, Apr 20, 2026 at 02:32:08PM +0300, Ido Schimmel wrote:
> > On Sun, Apr 19, 2026 at 10:53:32PM +0800, Ren Wei wrote:
> > > From: Haoze Xie <royenheart@gmail.com>
> > > 
> > > Several l3mdev slave-side helpers resolve an upper device and then use
> > > l3mdev_ops without first proving that the resolved device is still a
> > > valid L3 master.
> > > 
> > > During slave transition, an RCU reader can transiently observe an upper
> > > that is not an L3 master. Guard the affected slave-resolved paths by
> > > requiring the resolved upper to still be an L3 master before using
> > > l3mdev_ops, while keeping existing L3 RX handler providers intact.
> > > 
> > > Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
> > > Cc: stable@kernel.org
> > > 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>
> > > Tested-by: Haoze Xie <royenheart@gmail.com>
> > > Signed-off-by: Haoze Xie <royenheart@gmail.com>
> > > Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
> > 
> > I think it's fine for net:
> > 
> > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> 
> Thought about this again. I would like to check another approach
> (synchronize_net() after clearing IFF_L3MDEV_SLAVE). Will update
> tomorrow.

Sorry about the back and forth, but I thought about it again last night
and I think that this is a better fix:

https://github.com/idosch/linux/commit/e67517758ebcddf8a1b97817e4ab0fbf82f467fe.patch

It's a minimal fix in the control plane of the VRF driver which doesn't
add more checks in the data path. I can submit it later this week unless
there are objections.

Re: [PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers

[Yuan Tan]

On 4/21/2026 2:10 AM, Ido Schimmel wrote:
> On Mon, Apr 20, 2026 at 09:26:50PM +0300, Ido Schimmel wrote:
>> On Mon, Apr 20, 2026 at 02:32:08PM +0300, Ido Schimmel wrote:
>>> On Sun, Apr 19, 2026 at 10:53:32PM +0800, Ren Wei wrote:
>>>> From: Haoze Xie <royenheart@gmail.com>
>>>>
>>>> Several l3mdev slave-side helpers resolve an upper device and then use
>>>> l3mdev_ops without first proving that the resolved device is still a
>>>> valid L3 master.
>>>>
>>>> During slave transition, an RCU reader can transiently observe an upper
>>>> that is not an L3 master. Guard the affected slave-resolved paths by
>>>> requiring the resolved upper to still be an L3 master before using
>>>> l3mdev_ops, while keeping existing L3 RX handler providers intact.
>>>>
>>>> Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
>>>> Cc: stable@kernel.org
>>>> 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>
>>>> Tested-by: Haoze Xie <royenheart@gmail.com>
>>>> Signed-off-by: Haoze Xie <royenheart@gmail.com>
>>>> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
>>> I think it's fine for net:
>>>
>>> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
>> Thought about this again. I would like to check another approach
>> (synchronize_net() after clearing IFF_L3MDEV_SLAVE). Will update
>> tomorrow.
> Sorry about the back and forth, but I thought about it again last night
> and I think that this is a better fix:
>
> https://github.com/idosch/linux/commit/e67517758ebcddf8a1b97817e4ab0fbf82f467fe.patch
>
> It's a minimal fix in the control plane of the VRF driver which doesn't
> add more checks in the data path. I can submit it later this week unless
> there are objections.

Thank for you time and review!

Btw could you add a Reported-by: Yuan Tan <yuantan098@gmail.com>? Thanks.