[PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Ren Wei]

From: Longxuan Yu <ylong030@ucr.edu>

vlan_dev_set_egress_priority() leaves cleared egress priority mapping
nodes in the hash until device teardown. Repeated set/clear cycles with
distinct skb priorities therefore allocate an unbounded number of
vlan_priority_tci_mapping objects and leak memory.

Delete mappings when vlan_prio is cleared instead of keeping
tombstones. The TX fast path and reporting paths walk the lists without
RTNL, so convert the egress mapping lists to RCU-protected pointers and
defer freeing removed nodes until after a grace period.

Cc: stable@kernel.org
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
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: Longxuan Yu <ylong030@ucr.edu>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
 include/linux/if_vlan.h  | 23 +++++++++++--------
 net/8021q/vlan_dev.c     | 48 +++++++++++++++++++++++-----------------
 net/8021q/vlan_netlink.c |  9 +++-----
 net/8021q/vlanproc.c     | 12 ++++++----
 4 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index e6272f9c5e42..0a27533e70f3 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -151,7 +151,7 @@ extern __be16 vlan_dev_vlan_proto(const struct net_device *dev);
 struct vlan_priority_tci_mapping {
 	u32					priority;
 	u16					vlan_qos;
-	struct vlan_priority_tci_mapping	*next;
+	struct vlan_priority_tci_mapping __rcu	*next;
 };
 
 struct proc_dir_entry;
@@ -177,7 +177,7 @@ struct vlan_dev_priv {
 	unsigned int				nr_ingress_mappings;
 	u32					ingress_priority_map[8];
 	unsigned int				nr_egress_mappings;
-	struct vlan_priority_tci_mapping	*egress_priority_map[16];
+	struct vlan_priority_tci_mapping __rcu	*egress_priority_map[16];
 
 	__be16					vlan_proto;
 	u16					vlan_id;
@@ -209,19 +209,24 @@ static inline u16
 vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
 {
 	struct vlan_priority_tci_mapping *mp;
+	u16 vlan_qos = 0;
 
-	smp_rmb(); /* coupled with smp_wmb() in vlan_dev_set_egress_priority() */
+	rcu_read_lock();
 
-	mp = vlan_dev_priv(dev)->egress_priority_map[(skprio & 0xF)];
+	mp = rcu_dereference(vlan_dev_priv(dev)->egress_priority_map[skprio & 0xF]);
 	while (mp) {
 		if (mp->priority == skprio) {
-			return mp->vlan_qos; /* This should already be shifted
-					      * to mask correctly with the
-					      * VLAN's TCI */
+			vlan_qos = READ_ONCE(mp->vlan_qos);
+			break;
 		}
-		mp = mp->next;
+		mp = rcu_dereference(mp->next);
 	}
-	return 0;
+	rcu_read_unlock();
+
+	/* This should already be shifted to mask correctly with
+	 * the VLAN's TCI.
+	 */
+	return vlan_qos;
 }
 
 extern bool vlan_do_receive(struct sk_buff **skb);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c40f7d5c4fca..377616f51697 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -172,41 +172,43 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 				 u32 skb_prio, u16 vlan_prio)
 {
-	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	struct vlan_priority_tci_mapping *mp = NULL;
-	struct vlan_priority_tci_mapping *np;
-	u32 vlan_qos = (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
+	struct vlan_priority_tci_mapping __rcu **mpp;
+	struct vlan_priority_tci_mapping *mp;
+	struct vlan_priority_tci_mapping *np;
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	u32 bucket = skb_prio & 0xF;
+	u32 vlan_qos = (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
 
 	/* See if a priority mapping exists.. */
-	mp = vlan->egress_priority_map[skb_prio & 0xF];
+	mpp = &vlan->egress_priority_map[bucket];
+	mp = rtnl_dereference(*mpp);
 	while (mp) {
 		if (mp->priority == skb_prio) {
-			if (mp->vlan_qos && !vlan_qos)
+			if (!vlan_qos) {
+				rcu_assign_pointer(*mpp, rtnl_dereference(mp->next));
 				vlan->nr_egress_mappings--;
-			else if (!mp->vlan_qos && vlan_qos)
-				vlan->nr_egress_mappings++;
-			mp->vlan_qos = vlan_qos;
+				kfree_rcu_mightsleep(mp);
+			} else {
+				WRITE_ONCE(mp->vlan_qos, vlan_qos);
+			}
 			return 0;
 		}
-		mp = mp->next;
+		mpp = &mp->next;
+		mp = rtnl_dereference(*mpp);
 	}
 
 	/* Create a new mapping then. */
-	mp = vlan->egress_priority_map[skb_prio & 0xF];
+	if (!vlan_qos)
+		return 0;
+
 	np = kmalloc_obj(struct vlan_priority_tci_mapping);
 	if (!np)
 		return -ENOBUFS;
 
-	np->next = mp;
 	np->priority = skb_prio;
 	np->vlan_qos = vlan_qos;
-	/* Before inserting this element in hash table, make sure all its fields
-	 * are committed to memory.
-	 * coupled with smp_rmb() in vlan_dev_get_egress_qos_mask()
-	 */
-	smp_wmb();
-	vlan->egress_priority_map[skb_prio & 0xF] = np;
-	if (vlan_qos)
-		vlan->nr_egress_mappings++;
+	RCU_INIT_POINTER(np->next, rtnl_dereference(vlan->egress_priority_map[bucket]));
+	rcu_assign_pointer(vlan->egress_priority_map[bucket], np);
+	vlan->nr_egress_mappings++;
 	return 0;
 }
 
@@ -604,11 +606,17 @@ void vlan_dev_free_egress_priority(const struct net_device *dev)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
-		while ((pm = vlan->egress_priority_map[i]) != NULL) {
-			vlan->egress_priority_map[i] = pm->next;
-			kfree(pm);
+		pm = rtnl_dereference(vlan->egress_priority_map[i]);
+		RCU_INIT_POINTER(vlan->egress_priority_map[i], NULL);
+		while (pm) {
+			struct vlan_priority_tci_mapping *next;
+
+			next = rtnl_dereference(pm->next);
+			kfree_rcu_mightsleep(pm);
+			pm = next;
 		}
 	}
+	vlan->nr_egress_mappings = 0;
 }
 
 static void vlan_dev_uninit(struct net_device *dev)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index a000b1ef0520..bbe7cbd97939 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -260,13 +260,10 @@ static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
 			goto nla_put_failure;
 
 		for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
-			for (pm = vlan->egress_priority_map[i]; pm;
-			     pm = pm->next) {
-				if (!pm->vlan_qos)
-					continue;
-
+			for (pm = rtnl_dereference(vlan->egress_priority_map[i]); pm;
+			     pm = rtnl_dereference(pm->next)) {
 				m.from = pm->priority;
-				m.to   = (pm->vlan_qos >> 13) & 0x7;
+				m.to   = (READ_ONCE(pm->vlan_qos) >> 13) & 0x7;
 				if (nla_put(skb, IFLA_VLAN_QOS_MAPPING,
 					    sizeof(m), &m))
 					goto nla_put_failure;
diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c
index fa67374bda49..0e424e0895b7 100644
--- a/net/8021q/vlanproc.c
+++ b/net/8021q/vlanproc.c
@@ -262,15 +262,19 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 		   vlan->ingress_priority_map[7]);
 
 	seq_printf(seq, " EGRESS priority mappings: ");
+	rcu_read_lock();
 	for (i = 0; i < 16; i++) {
-		const struct vlan_priority_tci_mapping *mp
-			= vlan->egress_priority_map[i];
+		const struct vlan_priority_tci_mapping *mp =
+			rcu_dereference(vlan->egress_priority_map[i]);
 		while (mp) {
+			u16 vlan_qos = READ_ONCE(mp->vlan_qos);
+
 			seq_printf(seq, "%u:%d ",
-				   mp->priority, ((mp->vlan_qos >> 13) & 0x7));
-			mp = mp->next;
+				   mp->priority, ((vlan_qos >> 13) & 0x7));
+			mp = rcu_dereference(mp->next);
 		}
 	}
+	rcu_read_unlock();
 	seq_puts(seq, "\n");
 
 	return 0;
-- 
2.43.0

Re: [PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
8021q: free cleared egress QoS mappings safely

This commit converts the 802.1Q egress priority mapping lists to use
RCU-protected pointers instead of keeping tombstones. It deletes mappings
when vlan_prio is cleared, deferring the freeing of removed nodes until
after an RCU grace period.

> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index a000b1ef05206..bbe7cbd97939d 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -260,13 +260,10 @@ static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>  			goto nla_put_failure;
>  
>  		for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
> -			for (pm = vlan->egress_priority_map[i]; pm;
> -			     pm = pm->next) {
> -				if (!pm->vlan_qos)
> -					continue;
> -
> +			for (pm = rtnl_dereference(vlan->egress_priority_map[i]); pm;
> +			     pm = rtnl_dereference(pm->next)) {

Is it safe to use rtnl_dereference() for this traversal?

The vlan_fill_info() function implements the rtnl_link_ops->fill_info
callback, which is invoked by rtnl_dump_ifinfo() to service interface
netlink dumps. Modern netlink dumps can operate locklessly under
rcu_read_lock() without acquiring the RTNL lock.

Using rtnl_dereference() expands to rcu_dereference_protected(), which
might trigger a lockdep warning when the RTNL lock is not held.

Additionally, it omits the memory barriers needed for safe RCU reader
traversal. Without these barriers, concurrent updates from
vlan_dev_set_egress_priority() could cause the lockless reader to observe
stale or uninitialized list nodes.

Would using rcu_dereference_rtnl() or rcu_dereference() here be more
appropriate to ensure safe lockless traversal during netlink dumps?

>  				m.from = pm->priority;
> -				m.to   = (pm->vlan_qos >> 13) & 0x7;
> +				m.to   = (READ_ONCE(pm->vlan_qos) >> 13) & 0x7;
>  				if (nla_put(skb, IFLA_VLAN_QOS_MAPPING,
>  					    sizeof(m), &m))
>  					goto nla_put_failure;

Re: [PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Simon Horman]

On Mon, Apr 13, 2026 at 05:07:20PM +0800, Ren Wei wrote:
> From: Longxuan Yu <ylong030@ucr.edu>
> 
> vlan_dev_set_egress_priority() leaves cleared egress priority mapping
> nodes in the hash until device teardown. Repeated set/clear cycles with
> distinct skb priorities therefore allocate an unbounded number of
> vlan_priority_tci_mapping objects and leak memory.
> 
> Delete mappings when vlan_prio is cleared instead of keeping
> tombstones. The TX fast path and reporting paths walk the lists without
> RTNL, so convert the egress mapping lists to RCU-protected pointers and
> defer freeing removed nodes until after a grace period.
> 
> Cc: stable@kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 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: Longxuan Yu <ylong030@ucr.edu>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  include/linux/if_vlan.h  | 23 +++++++++++--------
>  net/8021q/vlan_dev.c     | 48 +++++++++++++++++++++++-----------------
>  net/8021q/vlan_netlink.c |  9 +++-----
>  net/8021q/vlanproc.c     | 12 ++++++----
>  4 files changed, 53 insertions(+), 39 deletions(-)

There is a lot of change here. And I'd suggest splitting the patch up into
(at least) two patches:

1. Convert mappings to use RCU
2. Fix bug

As is, the bug fix itself is difficult to isolate amongst the other changes.

Also, AI generated review suggests that this bug was introduced by commit
b020cb488586 ("[VLAN]: Keep track of number of QoS mappings"). If so,
it would be appropriate to use that commit in the Fixes tag.

-- 
pw-bot: changes-requested

Re: [PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Eric Dumazet]

On Mon, Apr 13, 2026 at 2:08 AM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Longxuan Yu <ylong030@ucr.edu>
>
> vlan_dev_set_egress_priority() leaves cleared egress priority mapping
> nodes in the hash until device teardown. Repeated set/clear cycles with
> distinct skb priorities therefore allocate an unbounded number of
> vlan_priority_tci_mapping objects and leak memory.
>
> Delete mappings when vlan_prio is cleared instead of keeping
> tombstones. The TX fast path and reporting paths walk the lists without
> RTNL, so convert the egress mapping lists to RCU-protected pointers and
> defer freeing removed nodes until after a grace period.
>
> Cc: stable@kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 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: Longxuan Yu <ylong030@ucr.edu>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  include/linux/if_vlan.h  | 23 +++++++++++--------
>  net/8021q/vlan_dev.c     | 48 +++++++++++++++++++++++-----------------
>  net/8021q/vlan_netlink.c |  9 +++-----
>  net/8021q/vlanproc.c     | 12 ++++++----

>
> @@ -604,11 +606,17 @@ void vlan_dev_free_egress_priority(const struct net_device *dev)
>         int i;
>
>         for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
> -               while ((pm = vlan->egress_priority_map[i]) != NULL) {
> -                       vlan->egress_priority_map[i] = pm->next;
> -                       kfree(pm);
> +               pm = rtnl_dereference(vlan->egress_priority_map[i]);
> +               RCU_INIT_POINTER(vlan->egress_priority_map[i], NULL);
> +               while (pm) {
> +                       struct vlan_priority_tci_mapping *next;
> +
> +                       next = rtnl_dereference(pm->next);
> +                       kfree_rcu_mightsleep(pm);

Please avoid kfree_rcu_mightsleep().

Embed  instead one rcu_head in the object.

> +                       pm = next;
>                 }
>         }
> +       vlan->nr_egress_mappings = 0;

Re: [PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Yuan Tan]

On 4/15/26 08:15, Simon Horman wrote:
> On Mon, Apr 13, 2026 at 05:07:20PM +0800, Ren Wei wrote:
>> From: Longxuan Yu <ylong030@ucr.edu>
>>
>> vlan_dev_set_egress_priority() leaves cleared egress priority mapping
>> nodes in the hash until device teardown. Repeated set/clear cycles with
>> distinct skb priorities therefore allocate an unbounded number of
>> vlan_priority_tci_mapping objects and leak memory.
>>
>> Delete mappings when vlan_prio is cleared instead of keeping
>> tombstones. The TX fast path and reporting paths walk the lists without
>> RTNL, so convert the egress mapping lists to RCU-protected pointers and
>> defer freeing removed nodes until after a grace period.
>>
>> Cc: stable@kernel.org
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> 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: Longxuan Yu <ylong030@ucr.edu>
>> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>> ---
>>  include/linux/if_vlan.h  | 23 +++++++++++--------
>>  net/8021q/vlan_dev.c     | 48 +++++++++++++++++++++++-----------------
>>  net/8021q/vlan_netlink.c |  9 +++-----
>>  net/8021q/vlanproc.c     | 12 ++++++----
>>  4 files changed, 53 insertions(+), 39 deletions(-)
> There is a lot of change here. And I'd suggest splitting the patch up into
> (at least) two patches:
>
> 1. Convert mappings to use RCU
> 2. Fix bug
>
> As is, the bug fix itself is difficult to isolate amongst the other changes.
>
> Also, AI generated review suggests that this bug was introduced by commit
> b020cb488586 ("[VLAN]: Keep track of number of QoS mappings"). If so,
> it would be appropriate to use that commit in the Fixes tag.
>
Thank you very much for your review and suggestions. We will try to
revise it in this direction.
May I ask whether we should include your “Suggested-by” tag in the patch?

Re: [PATCH net 1/1] 8021q: free cleared egress QoS mappings safely

[Simon Horman]

On Wed, Apr 15, 2026 at 10:35:19PM -0700, Yuan Tan wrote:
> 
> On 4/15/26 08:15, Simon Horman wrote:
> > On Mon, Apr 13, 2026 at 05:07:20PM +0800, Ren Wei wrote:
> >> From: Longxuan Yu <ylong030@ucr.edu>
> >>
> >> vlan_dev_set_egress_priority() leaves cleared egress priority mapping
> >> nodes in the hash until device teardown. Repeated set/clear cycles with
> >> distinct skb priorities therefore allocate an unbounded number of
> >> vlan_priority_tci_mapping objects and leak memory.
> >>
> >> Delete mappings when vlan_prio is cleared instead of keeping
> >> tombstones. The TX fast path and reporting paths walk the lists without
> >> RTNL, so convert the egress mapping lists to RCU-protected pointers and
> >> defer freeing removed nodes until after a grace period.
> >>
> >> Cc: stable@kernel.org
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> 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: Longxuan Yu <ylong030@ucr.edu>
> >> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> >> ---
> >>  include/linux/if_vlan.h  | 23 +++++++++++--------
> >>  net/8021q/vlan_dev.c     | 48 +++++++++++++++++++++++-----------------
> >>  net/8021q/vlan_netlink.c |  9 +++-----
> >>  net/8021q/vlanproc.c     | 12 ++++++----
> >>  4 files changed, 53 insertions(+), 39 deletions(-)
> > There is a lot of change here. And I'd suggest splitting the patch up into
> > (at least) two patches:
> >
> > 1. Convert mappings to use RCU
> > 2. Fix bug
> >
> > As is, the bug fix itself is difficult to isolate amongst the other changes.
> >
> > Also, AI generated review suggests that this bug was introduced by commit
> > b020cb488586 ("[VLAN]: Keep track of number of QoS mappings"). If so,
> > it would be appropriate to use that commit in the Fixes tag.
> >
> Thank you very much for your review and suggestions. We will try to
> revise it in this direction.
> May I ask whether we should include your “Suggested-by” tag in the patch?

I don't think you should include a Suggested-by tag.

My reasoning is that I'm only providing feedback and possible enhancements
to your approach.  While I think Suggested-by would be appropriate if I'd
suggested creating this patch in the first place.