[PATCH net v2 0/3] macsec: use rcu_work to fix crypto cleanup in softirq context

[PATCH net v2 0/3] macsec: use rcu_work to fix crypto cleanup in softirq context

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

crypto_free_aead() can internally call vunmap() (e.g. via dma_free_attrs()
in hardware crypto drivers like hisi_sec2), which must not be invoked from
softirq context. Both free_rxsa() and free_txsa() are RCU callbacks that
run in softirq, causing a kernel crash on affected hardware.

This series fixes the issue by deferring the actual cleanup to a workqueue
using rcu_work, which combines the RCU grace period and workqueue dispatch
into a single primitive.

Two design decisions worth noting:

1. rcu_work instead of schedule_work() + synchronize_rcu()

   An alternative would be to call schedule_work() directly from
   macsec_rxsa_put()/macsec_txsa_put(), then call synchronize_rcu() at
   the start of the work handler to replace the grace period previously
   provided by call_rcu(). However, synchronize_rcu() blocks the worker
   thread for the duration of a full RCU grace period. Under high SA
   churn (e.g. tearing down an interface with many SAs), each SA would
   occupy a worker thread while waiting, and multiple concurrent calls
   cannot share the same grace period — leading to unnecessary latency
   and resource waste.

   rcu_work uses call_rcu_hurry() internally, which is fully asynchronous:
   the worker thread is only dispatched after the grace period has elapsed,
   and multiple concurrent queue_rcu_work() calls naturally batch under the
   same grace period via the RCU subsystem's existing coalescing mechanism.

2. Dedicated workqueue instead of system_wq

   Using a dedicated workqueue (macsec_wq) allows macsec_exit() to drain
   exactly the work items belonging to this module — by calling
   destroy_workqueue() after rcu_barrier(). If system_wq were used,
   flush_scheduled_work() would drain all pending work items across the
   entire system, creating unnecessary coupling with unrelated subsystems
   and potentially causing unexpected delays. The dedicated workqueue
   provides a clean, contained teardown path.

Changes in v2:
- Use rcu_work instead of work_struct to avoid the manual RCU callback
  wrapper (suggested by Kuniyuki Iwashima)
- Introduce a dedicated workqueue and drain it properly in macsec_exit()
  to prevent potential crash on module unload (noted by Sabrina Dubroca)
- Extend the fix to TX SAs, which suffer from the same issue
  (noted by Sabrina Dubroca)
- Add Fixes tag (noted by Sabrina Dubroca)
- Split into three patches

Thanks to Kuniyuki Iwashima and Sabrina Dubroca for the review.

Link: https://lore.kernel.org/netdev/20260506100107.388184-1-alexjlzheng@tencent.com/T/#u

Jinliang Zheng (3):
  macsec: introduce dedicated workqueue for SA crypto cleanup
  macsec: use rcu_work to defer RX SA crypto cleanup out of softirq
  macsec: use rcu_work to defer TX SA crypto cleanup out of softirq

 drivers/net/macsec.c | 27 ++++++++++++++++++++-------
 include/net/macsec.h |  5 +++--
 2 files changed, 23 insertions(+), 9 deletions(-)

-- 
2.39.3

[PATCH net v2 v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

Introduce a dedicated ordered workqueue, macsec_wq, which will be used
by subsequent patches to defer SA crypto cleanup (crypto_free_aead and
related teardown) out of softirq context.

Using a dedicated workqueue instead of system_wq allows macsec_exit()
to drain exactly the work items belonging to this module via
destroy_workqueue(), without interfering with unrelated work items on
system_wq or causing unexpected delays elsewhere.

rcu_barrier() in macsec_exit() ensures all in-flight rcu_work callbacks
have enqueued their work items before destroy_workqueue() drains and
destroys the queue, making the two-step teardown correct and complete.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 drivers/net/macsec.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f6cad0746a02..ddb22473e701 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -26,6 +26,8 @@
 
 #include <uapi/linux/if_macsec.h>
 
+static struct workqueue_struct *macsec_wq;
+
 /* SecTAG length = macsec_eth_header without the optional SCI */
 #define MACSEC_TAG_LEN 6
 
@@ -4450,10 +4452,14 @@ static int __init macsec_init(void)
 {
 	int err;
 
+	macsec_wq = alloc_ordered_workqueue("macsec", 0);
+	if (!macsec_wq)
+		return -ENOMEM;
+
 	pr_info("MACsec IEEE 802.1AE\n");
 	err = register_netdevice_notifier(&macsec_notifier);
 	if (err)
-		return err;
+		goto wq;
 
 	err = rtnl_link_register(&macsec_link_ops);
 	if (err)
@@ -4469,6 +4475,8 @@ static int __init macsec_init(void)
 	rtnl_link_unregister(&macsec_link_ops);
 notifier:
 	unregister_netdevice_notifier(&macsec_notifier);
+wq:
+	destroy_workqueue(macsec_wq);
 	return err;
 }
 
@@ -4478,6 +4486,7 @@ static void __exit macsec_exit(void)
 	rtnl_link_unregister(&macsec_link_ops);
 	unregister_netdevice_notifier(&macsec_notifier);
 	rcu_barrier();
+	destroy_workqueue(macsec_wq);
 }
 
 module_init(macsec_init);
-- 
2.39.3

Re: [PATCH net v2 v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup

[Jakub Kicinski]

On Thu,  7 May 2026 10:04:24 +0800 alexjlzheng@gmail.com wrote:
> +wq:

nit: better name:

err_destroy_wq:

> +	destroy_workqueue(macsec_wq);

Sashiko implies that rcu_barrier() is needed before this 

>  	return err;

Re: [PATCH net v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

On Fri, 8 May 2026 15:42:43 -0700, Jakub Kicinski wrote:
> > +wq:
> > +	destroy_workqueue(macsec_wq);
> > +	return err;
>
> nit: err_destroy_wq: would be a better label name
>
> rcu_barrier() is needed before this

Thanks for the review.

Regarding the label name: "wq:" follows the existing naming convention
in macsec_init(), where the other error labels are also named after the
resource they clean up ("rtnl:", "notifier:") rather than using the
"err_xxx:" style. I kept it consistent with that local convention, but
happy to switch to "err_destroy_wq:" if you prefer uniformity with the
broader codebase.

Regarding rcu_barrier(): in the error path of macsec_init(), the
workqueue has just been created and no MACsec interface has been
registered yet. queue_rcu_work(macsec_wq, ...) is only reachable
from macsec_rxsa_put() and macsec_txsa_put(), which require an SA
object to exist. No SA can be created before macsec_init() succeeds,
so macsec_wq is guaranteed to be empty at this point and
rcu_barrier() is not needed here.

The rcu_barrier() in macsec_exit() is necessary for a different
reason: at that point live interfaces may have been destroyed and
their SAs may be in-flight through the rcu_work mechanism, so we
must wait for all rcu_work_rcufn callbacks to finish enqueuing their
work items before destroy_workqueue() drains them.

Does that reasoning make sense, or am I missing a path where work
could be queued onto macsec_wq during the init error unwind?

Thanks,
Jinliang

Re: [PATCH net v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup

[Jakub Kicinski]

On Sat,  9 May 2026 10:00:54 +0800 alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
> 
> On Fri, 8 May 2026 15:42:43 -0700, Jakub Kicinski wrote:
> > > +wq:
> > > +	destroy_workqueue(macsec_wq);
> > > +	return err;  
> >
> > nit: err_destroy_wq: would be a better label name
> >
> > rcu_barrier() is needed before this  
> 
> Thanks for the review.
> 
> Regarding the label name: "wq:" follows the existing naming convention
> in macsec_init(), where the other error labels are also named after the
> resource they clean up ("rtnl:", "notifier:") rather than using the
> "err_xxx:" style. I kept it consistent with that local convention, but
> happy to switch to "err_destroy_wq:" if you prefer uniformity with the
> broader codebase.

I can see that, but we have to start somewhere.

> Regarding rcu_barrier(): in the error path of macsec_init(), the
> workqueue has just been created and no MACsec interface has been
> registered yet.  queue_rcu_work(macsec_wq, ...) is only reachable
> from macsec_rxsa_put() and macsec_txsa_put(), which require an SA
> object to exist. No SA can be created before macsec_init() succeeds,
> so macsec_wq is guaranteed to be empty at this point and
> rcu_barrier() is not needed here.
> 
> The rcu_barrier() in macsec_exit() is necessary for a different
> reason: at that point live interfaces may have been destroyed and
> their SAs may be in-flight through the rcu_work mechanism, so we
> must wait for all rcu_work_rcufn callbacks to finish enqueuing their
> work items before destroy_workqueue() drains them.
> 
> Does that reasoning make sense, or am I missing a path where work
> could be queued onto macsec_wq during the init error unwind?

TBH I didn't check the code, you may be right that until the genl
family is registered there's no way to an SA to exist (even tho
macsec devices may have already gotten created).

Either way, I'd just add that barrier. We're basically leaving
a trap for whoever tries to add another piece of code to this function.
The error unwind path and the remove path should generally be kept
the same.

Re: [PATCH net v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

On Thu, 8 May 2026 19:19:23 -0700, Jakub Kicinski wrote:
> > Regarding the label name: "wq:" follows the existing naming convention
> > in macsec_init(), where the other error labels are also named after the
> > resource they clean up ("rtnl:", "notifier:") rather than using the
> > "err_xxx:" style.
>
> I can see that, but we have to start somewhere.

Good point, will rename to err_destroy_wq: in v3.

> > Regarding rcu_barrier(): in the error path of macsec_init(), the
> > workqueue has just been created and no MACsec interface has been
> > registered yet. [...]
>
> TBH I didn't check the code, you may be right that until the genl
> family is registered there's no way to an SA to exist (even tho
> macsec devices may have already gotten created).
>
> Either way, I'd just add that barrier. We're basically leaving
> a trap for whoever tries to add another piece of code to this function.
> The error unwind path and the remove path should generally be kept
> the same.

Agreed, that's a good point. Keeping the error unwind consistent with
the remove path avoids subtle future bugs. Will add rcu_barrier() before
destroy_workqueue() in the error path in v3.

Thanks for the review!

Jinliang

[PATCH net v2 v2 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

crypto_free_aead() can internally invoke vunmap() (e.g. via
dma_free_attrs() in hardware crypto drivers such as hisi_sec2).
vunmap() must not be called from softirq context, but free_rxsa()
is an RCU callback that runs in softirq, leading to a kernel crash:

  vunmap+0x4c/0x70
  __iommu_dma_free+0xd0/0x138
  dma_free_attrs+0xf4/0x100
  sec_aead_exit+0x64/0xb8 [hisi_sec2]
  crypto_destroy_tfm+0x98/0x110
  free_rxsa+0x28/0x50 [macsec]
  rcu_do_batch+0x184/0x460
  rcu_core+0xf4/0x1f8
  handle_softirqs+0x118/0x330

Use rcu_work to defer the cleanup to a workqueue. rcu_work dispatches
the worker asynchronously after the RCU grace period, so no thread
blocks waiting, and concurrent releases of multiple SAs naturally
share the same grace period.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 drivers/net/macsec.c | 8 +++++---
 include/net/macsec.h | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index ddb22473e701..e0862ecd23f2 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -176,9 +176,10 @@ static void macsec_rxsc_put(struct macsec_rx_sc *sc)
 		call_rcu(&sc->rcu_head, free_rx_sc_rcu);
 }
 
-static void free_rxsa(struct rcu_head *head)
+static void free_rxsa_work(struct work_struct *work)
 {
-	struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu);
+	struct macsec_rx_sa *sa =
+		container_of(to_rcu_work(work), struct macsec_rx_sa, destroy_work);
 
 	crypto_free_aead(sa->key.tfm);
 	free_percpu(sa->stats);
@@ -188,7 +189,7 @@ static void free_rxsa(struct rcu_head *head)
 static void macsec_rxsa_put(struct macsec_rx_sa *sa)
 {
 	if (refcount_dec_and_test(&sa->refcnt))
-		call_rcu(&sa->rcu, free_rxsa);
+		queue_rcu_work(macsec_wq, &sa->destroy_work);
 }
 
 static struct macsec_tx_sa *macsec_txsa_get(struct macsec_tx_sa __rcu *ptr)
@@ -1409,6 +1410,7 @@ static int init_rx_sa(struct macsec_rx_sa *rx_sa, char *sak, int key_len,
 	rx_sa->next_pn = 1;
 	refcount_set(&rx_sa->refcnt, 1);
 	spin_lock_init(&rx_sa->lock);
+	INIT_RCU_WORK(&rx_sa->destroy_work, free_rxsa_work);
 
 	return 0;
 }
diff --git a/include/net/macsec.h b/include/net/macsec.h
index bc7de5b53e54..d6136debe1c8 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -9,6 +9,7 @@
 
 #include <linux/u64_stats_sync.h>
 #include <linux/if_vlan.h>
+#include <linux/workqueue.h>
 #include <uapi/linux/if_link.h>
 #include <uapi/linux/if_macsec.h>
 
@@ -136,7 +137,7 @@ struct macsec_rx_sa {
 	bool active;
 	struct macsec_rx_sa_stats __percpu *stats;
 	struct macsec_rx_sc *sc;
-	struct rcu_head rcu;
+	struct rcu_work destroy_work;
 };
 
 struct pcpu_rx_sc_stats {
-- 
2.39.3

Re: [PATCH net v2 v2 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq

[Jakub Kicinski]

On Thu,  7 May 2026 10:04:25 +0800 alexjlzheng@gmail.com wrote:
> @@ -136,7 +137,7 @@ struct macsec_rx_sa {
>  	bool active;
>  	struct macsec_rx_sa_stats __percpu *stats;
>  	struct macsec_rx_sc *sc;
> -	struct rcu_head rcu;
> +	struct rcu_work destroy_work;

Please add a kdoc for the new field? (same in patch 3)
-- 
pw-bot: cr

[PATCH net v2 v2 3/3] macsec: use rcu_work to defer TX SA crypto cleanup out of softirq

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

free_txsa() is an RCU callback running in softirq context, but calls
crypto_free_aead() which can invoke vunmap() internally on hardware
crypto drivers (e.g. hisi_sec2), triggering a kernel crash.

Use rcu_work to defer the cleanup to a workqueue, for the same reasons
as the analogous fix to free_rxsa() in the previous patch.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 drivers/net/macsec.c | 8 +++++---
 include/net/macsec.h | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index e0862ecd23f2..c3eefeef9526 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -205,9 +205,10 @@ static struct macsec_tx_sa *macsec_txsa_get(struct macsec_tx_sa __rcu *ptr)
 	return sa;
 }
 
-static void free_txsa(struct rcu_head *head)
+static void free_txsa_work(struct work_struct *work)
 {
-	struct macsec_tx_sa *sa = container_of(head, struct macsec_tx_sa, rcu);
+	struct macsec_tx_sa *sa =
+		container_of(to_rcu_work(work), struct macsec_tx_sa, destroy_work);
 
 	crypto_free_aead(sa->key.tfm);
 	free_percpu(sa->stats);
@@ -217,7 +218,7 @@ static void free_txsa(struct rcu_head *head)
 static void macsec_txsa_put(struct macsec_tx_sa *sa)
 {
 	if (refcount_dec_and_test(&sa->refcnt))
-		call_rcu(&sa->rcu, free_txsa);
+		queue_rcu_work(macsec_wq, &sa->destroy_work);
 }
 
 static struct macsec_cb *macsec_skb_cb(struct sk_buff *skb)
@@ -1510,6 +1511,7 @@ static int init_tx_sa(struct macsec_tx_sa *tx_sa, char *sak, int key_len,
 	tx_sa->active = false;
 	refcount_set(&tx_sa->refcnt, 1);
 	spin_lock_init(&tx_sa->lock);
+	INIT_RCU_WORK(&tx_sa->destroy_work, free_txsa_work);
 
 	return 0;
 }
diff --git a/include/net/macsec.h b/include/net/macsec.h
index d6136debe1c8..cf92f57733df 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -187,7 +187,7 @@ struct macsec_tx_sa {
 	refcount_t refcnt;
 	bool active;
 	struct macsec_tx_sa_stats __percpu *stats;
-	struct rcu_head rcu;
+	struct rcu_work destroy_work;
 };
 
 /**
-- 
2.39.3