The backport of commit 21032425c36f ("net: vlan: fix a UAF in vlan_dev_real_dev()") in v5.15.3 introduced a netdevice refcount leak that was fixed upstream.
The first commit is trivial and helps to limit conflicts when applying the second commit, which fixes the issue.
The last conflict is solved by using dev_put() instead of dev_put_track(), as the latter was introduced in 5.17 and does not exist in 5.15.x.
Xin Long (2): vlan: introduce vlan_dev_free_egress_priority vlan: move dev_put into vlan_dev_uninit
net/8021q/vlan.h | 2 +- net/8021q/vlan_dev.c | 15 +++++++++++---- net/8021q/vlan_netlink.c | 7 ++++--- 3 files changed, 16 insertions(+), 8 deletions(-)
From: Xin Long lucien.xin@gmail.com
commit 37aa50c539bcbcc01767e515bd170787fcfc0f33 upstream.
This patch is to introduce vlan_dev_free_egress_priority() to free egress priority for vlan dev, and keep vlan_dev_uninit() static as .ndo_uninit. It makes the code more clear and safer when adding new code in vlan_dev_uninit() in the future.
Signed-off-by: Xin Long lucien.xin@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Olivier Matz olivier.matz@6wind.com --- net/8021q/vlan.h | 2 +- net/8021q/vlan_dev.c | 7 ++++++- net/8021q/vlan_netlink.c | 7 ++++--- 3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h index 1a705a4ef7fa..5eaf38875554 100644 --- a/net/8021q/vlan.h +++ b/net/8021q/vlan.h @@ -129,6 +129,7 @@ void vlan_dev_set_ingress_priority(const struct net_device *dev, u32 skb_prio, u16 vlan_prio); int vlan_dev_set_egress_priority(const struct net_device *dev, u32 skb_prio, u16 vlan_prio); +void vlan_dev_free_egress_priority(const struct net_device *dev); int vlan_dev_change_flags(const struct net_device *dev, u32 flag, u32 mask); void vlan_dev_get_realdev_name(const struct net_device *dev, char *result, size_t size); @@ -139,7 +140,6 @@ int vlan_check_real_dev(struct net_device *real_dev, void vlan_setup(struct net_device *dev); int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack); void unregister_vlan_dev(struct net_device *dev, struct list_head *head); -void vlan_dev_uninit(struct net_device *dev); bool vlan_dev_inherit_address(struct net_device *dev, struct net_device *real_dev);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index 3d0f0d0a323b..cf66be83fb77 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -622,7 +622,7 @@ static int vlan_dev_init(struct net_device *dev) }
/* Note: this function might be called multiple times for the same device. */ -void vlan_dev_uninit(struct net_device *dev) +void vlan_dev_free_egress_priority(const struct net_device *dev) { struct vlan_priority_tci_mapping *pm; struct vlan_dev_priv *vlan = vlan_dev_priv(dev); @@ -636,6 +636,11 @@ void vlan_dev_uninit(struct net_device *dev) } }
+static void vlan_dev_uninit(struct net_device *dev) +{ + vlan_dev_free_egress_priority(dev); +} + static netdev_features_t vlan_dev_fix_features(struct net_device *dev, netdev_features_t features) { diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c index 0db85aeb119b..53b1955b027f 100644 --- a/net/8021q/vlan_netlink.c +++ b/net/8021q/vlan_netlink.c @@ -183,10 +183,11 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev, return -EINVAL;
err = vlan_changelink(dev, tb, data, extack); - if (!err) - err = register_vlan_dev(dev, extack); if (err) - vlan_dev_uninit(dev); + return err; + err = register_vlan_dev(dev, extack); + if (err) + vlan_dev_free_egress_priority(dev); return err; }
From: Xin Long lucien.xin@gmail.com
commit d6ff94afd90b0ce8d1715f8ef77d4347d7a7f2c0 upstream.
Shuang Li reported an QinQ issue by simply doing:
# ip link add dummy0 type dummy # ip link add link dummy0 name dummy0.1 type vlan id 1 # ip link add link dummy0.1 name dummy0.1.2 type vlan id 2 # rmmod 8021q
unregister_netdevice: waiting for dummy0.1 to become free. Usage count = 1
When rmmods 8021q, all vlan devs are deleted from their real_dev's vlan grp and added into list_kill by unregister_vlan_dev(). dummy0.1 is unregistered before dummy0.1.2, as it's using for_each_netdev() in __rtnl_kill_links().
When unregisters dummy0.1, dummy0.1.2 is not unregistered in the event of NETDEV_UNREGISTER, as it's been deleted from dummy0.1's vlan grp. However, due to dummy0.1.2 still holding dummy0.1, dummy0.1 will keep waiting in netdev_wait_allrefs(), while dummy0.1.2 will never get unregistered and release dummy0.1, as it delays dev_put until calling dev->priv_destructor, vlan_dev_free().
This issue was introduced by Commit 563bcbae3ba2 ("net: vlan: fix a UAF in vlan_dev_real_dev()"), and this patch is to fix it by moving dev_put() into vlan_dev_uninit(), which is called after NETDEV_UNREGISTER event but before netdev_wait_allrefs().
Fixes: 563bcbae3ba2 ("net: vlan: fix a UAF in vlan_dev_real_dev()") Reported-by: Shuang Li shuali@redhat.com Signed-off-by: Xin Long lucien.xin@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Olivier Matz olivier.matz@6wind.com --- net/8021q/vlan_dev.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index cf66be83fb77..ad2d3ad34b7d 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -638,7 +638,12 @@ void vlan_dev_free_egress_priority(const struct net_device *dev)
static void vlan_dev_uninit(struct net_device *dev) { + struct vlan_dev_priv *vlan = vlan_dev_priv(dev); + vlan_dev_free_egress_priority(dev); + + /* Get rid of the vlan's reference to real_dev */ + dev_put(vlan->real_dev); }
static netdev_features_t vlan_dev_fix_features(struct net_device *dev, @@ -851,9 +856,6 @@ static void vlan_dev_free(struct net_device *dev)
free_percpu(vlan->vlan_pcpu_stats); vlan->vlan_pcpu_stats = NULL; - - /* Get rid of the vlan's reference to real_dev */ - dev_put(vlan->real_dev); }
void vlan_setup(struct net_device *dev)
On Fri, Dec 01, 2023 at 02:30:02PM +0100, Olivier Matz wrote:
The backport of commit 21032425c36f ("net: vlan: fix a UAF in vlan_dev_real_dev()") in v5.15.3 introduced a netdevice refcount leak that was fixed upstream.
The first commit is trivial and helps to limit conflicts when applying the second commit, which fixes the issue.
The last conflict is solved by using dev_put() instead of dev_put_track(), as the latter was introduced in 5.17 and does not exist in 5.15.x.
Xin Long (2): vlan: introduce vlan_dev_free_egress_priority vlan: move dev_put into vlan_dev_uninit
net/8021q/vlan.h | 2 +- net/8021q/vlan_dev.c | 15 +++++++++++---- net/8021q/vlan_netlink.c | 7 ++++--- 3 files changed, 16 insertions(+), 8 deletions(-)
-- 2.30.2
Both now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org