[PATCH 5.15 0/2] vlan: fix netdev refcount leak

public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 5.15 0/2] vlan: fix netdev refcount leak
@ 2023-12-01 13:30 Olivier Matz
  2023-12-01 13:30 ` [PATCH 5.15 1/2] vlan: introduce vlan_dev_free_egress_priority Olivier Matz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Olivier Matz @ 2023-12-01 13:30 UTC (permalink / raw)
  To: stable

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

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

* [PATCH 5.15 1/2] vlan: introduce vlan_dev_free_egress_priority
  2023-12-01 13:30 [PATCH 5.15 0/2] vlan: fix netdev refcount leak Olivier Matz
@ 2023-12-01 13:30 ` Olivier Matz
  2023-12-01 13:30 ` [PATCH 5.15 2/2] vlan: move dev_put into vlan_dev_uninit Olivier Matz
  2023-12-03 13:56 ` [PATCH 5.15 0/2] vlan: fix netdev refcount leak Greg KH
  2 siblings, 0 replies; 4+ messages in thread
From: Olivier Matz @ 2023-12-01 13:30 UTC (permalink / raw)
  To: stable; +Cc: Xin Long, David S . Miller

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;
 }
 
-- 
2.30.2


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

* [PATCH 5.15 2/2] vlan: move dev_put into vlan_dev_uninit
  2023-12-01 13:30 [PATCH 5.15 0/2] vlan: fix netdev refcount leak Olivier Matz
  2023-12-01 13:30 ` [PATCH 5.15 1/2] vlan: introduce vlan_dev_free_egress_priority Olivier Matz
@ 2023-12-01 13:30 ` Olivier Matz
  2023-12-03 13:56 ` [PATCH 5.15 0/2] vlan: fix netdev refcount leak Greg KH
  2 siblings, 0 replies; 4+ messages in thread
From: Olivier Matz @ 2023-12-01 13:30 UTC (permalink / raw)
  To: stable; +Cc: Xin Long, Shuang Li, David S . Miller

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

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

* Re: [PATCH 5.15 0/2] vlan: fix netdev refcount leak
  2023-12-01 13:30 [PATCH 5.15 0/2] vlan: fix netdev refcount leak Olivier Matz
  2023-12-01 13:30 ` [PATCH 5.15 1/2] vlan: introduce vlan_dev_free_egress_priority Olivier Matz
  2023-12-01 13:30 ` [PATCH 5.15 2/2] vlan: move dev_put into vlan_dev_uninit Olivier Matz
@ 2023-12-03 13:56 ` Greg KH
  2 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2023-12-03 13:56 UTC (permalink / raw)
  To: Olivier Matz; +Cc: stable

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

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

end of thread, other threads:[~2023-12-03 13:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-01 13:30 [PATCH 5.15 0/2] vlan: fix netdev refcount leak Olivier Matz
2023-12-01 13:30 ` [PATCH 5.15 1/2] vlan: introduce vlan_dev_free_egress_priority Olivier Matz
2023-12-01 13:30 ` [PATCH 5.15 2/2] vlan: move dev_put into vlan_dev_uninit Olivier Matz
2023-12-03 13:56 ` [PATCH 5.15 0/2] vlan: fix netdev refcount leak Greg KH

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