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

[PATCH net v3 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 v3:
- Add rcu_barrier() before destroy_workqueue() in the error path of
  macsec_init() as a precaution, mirroring macsec_exit() to stay safe
  if work ever becomes queueable earlier (suggested by Jakub Kicinski)
- Rename error labels in macsec_init() from resource-named style
  (rtnl:, notifier:, wq:) to err_xxx: style (suggested by Jakub Kicinski)
- Add kdoc for the new destroy_work field in struct macsec_rx_sa and
  struct macsec_tx_sa (suggested by Jakub Kicinski)

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 Jakub Kicinski, Kuniyuki Iwashima, and Sabrina Dubroca for
the review.

Link: https://lore.kernel.org/netdev/20260506100107.388184-1-alexjlzheng@tencent.com/T/#u
Link: https://lore.kernel.org/netdev/20260509020054.1792674-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 | 39 ++++++++++++++++++++++++++++-----------
 include/net/macsec.h |  7 +++++--
 2 files changed, 33 insertions(+), 13 deletions(-)

-- 
2.39.3

[PATCH net v3 v3 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.
The same sequence is kept in the error path of macsec_init() as a
precaution, to mirror macsec_exit() and stay safe if work ever becomes
queueable before this point in the future.

While at it, rename the error labels in macsec_init() from the
resource-named style (rtnl:, notifier:, wq:) to the err_xxx: style
(err_rtnl:, err_notifier:, err_destroy_wq:) to align with the broader
kernel convention.

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

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f6cad0746a02..52d6dd03ac92 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,25 +4452,35 @@ 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 err_destroy_wq;
 
 	err = rtnl_link_register(&macsec_link_ops);
 	if (err)
-		goto notifier;
+		goto err_notifier;
 
 	err = genl_register_family(&macsec_fam);
 	if (err)
-		goto rtnl;
+		goto err_rtnl;
 
 	return 0;
 
-rtnl:
+err_rtnl:
 	rtnl_link_unregister(&macsec_link_ops);
-notifier:
+err_notifier:
 	unregister_netdevice_notifier(&macsec_notifier);
+err_destroy_wq:
+	/* Precautionary, mirrors macsec_exit() to stay safe if work
+	 * ever becomes queueable before this point in the future.
+	 */
+	rcu_barrier();
+	destroy_workqueue(macsec_wq);
 	return err;
 }
 
@@ -4478,6 +4490,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

[PATCH net v3 v3 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 | 4 +++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 52d6dd03ac92..67e4e3f439c7 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..0980ef36fbf0 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>
 
@@ -123,6 +124,7 @@ struct macsec_dev_stats {
  * @key: key structure
  * @ssci: short secure channel identifier
  * @stats: per-SA stats
+ * @destroy_work: deferred work to free the SA in process context after RCU grace period
  */
 struct macsec_rx_sa {
 	struct macsec_key key;
@@ -136,7 +138,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

[PATCH net v3 v3 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 | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 67e4e3f439c7..e91d853c8d45 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 0980ef36fbf0..d962093ee923 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -176,6 +176,7 @@ struct macsec_rx_sc {
  * @key: key structure
  * @ssci: short secure channel identifier
  * @stats: per-SA stats
+ * @destroy_work: deferred work to free the SA in process context after RCU grace period
  */
 struct macsec_tx_sa {
 	struct macsec_key key;
@@ -188,7 +189,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

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

[Sabrina Dubroca]

Thanks for the updated patch. A few minor comments:

2026-05-09, 11:33:45 +0800, alexjlzheng@gmail.com wrote:
> @@ -4450,25 +4452,35 @@ static int __init macsec_init(void)
>  {
>  	int err;
>  
> +	macsec_wq = alloc_ordered_workqueue("macsec", 0);
> +	if (!macsec_wq)
> +		return -ENOMEM;

Why did you pick an ordered workqueue instead of a normal one?

[...]
> -rtnl:
> +err_rtnl:
>  	rtnl_link_unregister(&macsec_link_ops);
> -notifier:
> +err_notifier:
>  	unregister_netdevice_notifier(&macsec_notifier);
> +err_destroy_wq:
> +	/* Precautionary, mirrors macsec_exit() to stay safe if work
> +	 * ever becomes queueable before this point in the future.
> +	 */

I don't think this comment is really necessary, but ok.

(I missed the discussion on v2, it's not possible to create SAs at
this point, but it's reasonable to add the rcu_barrier here)

> +	rcu_barrier();
> +	destroy_workqueue(macsec_wq);
>  	return err;
>  }

-- 
Sabrina

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

[Sabrina Dubroca]

2026-05-09, 11:33:46 +0800, alexjlzheng@gmail.com wrote:
> 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 | 4 +++-
>  2 files changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

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

[Sabrina Dubroca]

2026-05-09, 11:33:47 +0800, alexjlzheng@gmail.com wrote:
> 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 | 3 ++-
>  2 files changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

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

[Jinliang Zheng]

On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > +	macsec_wq = alloc_ordered_workqueue("macsec", 0);
>
> Why did you pick an ordered workqueue instead of a normal one?

I chose an ordered workqueue for its serialized execution, which
avoids potential ordering issues between concurrent SA destructions.
In hindsight, there is no actual ordering dependency between SA
cleanups, so a normal workqueue is sufficient here.

Should I send a v4 with that switched to alloc_workqueue()?

Thanks,
Jinliang

> > +err_destroy_wq:
> > +	/* Precautionary, mirrors macsec_exit() to stay safe if work
> > +	 * ever becomes queueable before this point in the future.
> > +	 */
>
> I don't think this comment is really necessary, but ok.
>
> (I missed the discussion on v2, it's not possible to create SAs at
> this point, but it's reasonable to add the rcu_barrier here)

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

[Sabrina Dubroca]

2026-05-11, 22:00:05 +0800, Jinliang Zheng wrote:
> On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > > +	macsec_wq = alloc_ordered_workqueue("macsec", 0);
> >
> > Why did you pick an ordered workqueue instead of a normal one?
> 
> I chose an ordered workqueue for its serialized execution, which
> avoids potential ordering issues between concurrent SA destructions.
> In hindsight, there is no actual ordering dependency between SA
> cleanups, so a normal workqueue is sufficient here.
> 
> Should I send a v4 with that switched to alloc_workqueue()?

Probably doesn't really matter, but since your cover letter mentions
"unnecessary latency", maybe? Would it make a difference in your use
case? I think it would mostly help when we try to unload the module
with lots of SAs, otherwise the only difference will be that actual
freeing of resources will be a bit slower. (OTOH it would avoid giving
the "wrong impression", that the ordered variant of the WQ is needed,
when it's not)


Either way, the patch looks ok to me, so for this version:

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

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

[Jinliang Zheng]

On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > In hindsight, there is no actual ordering dependency between SA
> > cleanups, so a normal workqueue is sufficient here. Should I send
> > a v4 with that switched to alloc_workqueue()?
>
> It probably doesn't matter a lot, but since your cover letter
> mentions "unnecessary latency", switching to a regular workqueue
> would help avoid giving the wrong impression that the ordered
> variant is needed.
>
> Either way, the patch looks ok to me
>
> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

Great, I'll switch to alloc_workqueue() in v4. Thanks a lot for
the review and the Reviewed-by!

Thanks,
Jinliang