[PATCH] macsec: defer RX SA cleanup from RCU callback to workqueue

[PATCH] macsec: defer RX SA cleanup from RCU callback to workqueue

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

crypto_free_aead() can call vunmap() internally (e.g. via
dma_free_attrs() in hardware crypto drivers like hisi_sec2), which
must not be called from softirq context.

free_rxsa() is an RCU callback and therefore runs in softirq context,
causing a kernel crash when the underlying AEAD implementation
performs DMA unmapping during tfm destruction:

  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

Fix this by splitting free_rxsa() into two parts: the RCU callback
now only schedules a work item, and the actual resource release
(crypto_free_aead, free_percpu, kfree) is done in a workqueue
handler running in process context.

Add a destroy_work field to struct macsec_rx_sa and initialize it
in init_rx_sa().

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 drivers/net/macsec.c | 13 +++++++++++--
 include/net/macsec.h |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f6cad0746a02..dabd3d2598ae 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -174,15 +174,23 @@ 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(work, struct macsec_rx_sa,
+					       destroy_work);
 
 	crypto_free_aead(sa->key.tfm);
 	free_percpu(sa->stats);
 	kfree(sa);
 }
 
+static void free_rxsa(struct rcu_head *head)
+{
+	struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu);
+
+	schedule_work(&sa->destroy_work);
+}
+
 static void macsec_rxsa_put(struct macsec_rx_sa *sa)
 {
 	if (refcount_dec_and_test(&sa->refcnt))
@@ -1407,6 +1415,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_WORK(&rx_sa->destroy_work, free_rxsa_work);
 
 	return 0;
 }
diff --git a/include/net/macsec.h b/include/net/macsec.h
index bc7de5b53e54..aeacd361f686 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>
 
@@ -137,6 +138,7 @@ struct macsec_rx_sa {
 	struct macsec_rx_sa_stats __percpu *stats;
 	struct macsec_rx_sc *sc;
 	struct rcu_head rcu;
+	struct work_struct destroy_work;
 };
 
 struct pcpu_rx_sc_stats {
-- 
2.39.3

Re: [PATCH] macsec: defer RX SA cleanup from RCU callback to workqueue

[albin_yang]

On Wed, May 06, 2026 at 10:01:07AM +0800, alexjlzheng@tencent.com wrote:
> crypto_free_aead() can call vunmap() internally (e.g. via
> dma_free_attrs() in hardware crypto drivers like hisi_sec2), which
> must not be called from softirq context.
> 
> free_rxsa() is an RCU callback and therefore runs in softirq context,
> causing a kernel crash when the underlying AEAD implementation
> performs DMA unmapping during tfm destruction:
> 
> [...]

Looks good to me.

Reviewed-by: Wei Yang <albinwyang@tencent.com>

Re: [PATCH] macsec: defer RX SA cleanup from RCU callback to workqueue

[Kuniyuki Iwashima]

From: alexjlzheng@gmail.com
Date: Wed,  6 May 2026 18:01:07 +0800
> From: Jinliang Zheng <alexjlzheng@tencent.com>
> 
> crypto_free_aead() can call vunmap() internally (e.g. via
> dma_free_attrs() in hardware crypto drivers like hisi_sec2), which
> must not be called from softirq context.
> 
> free_rxsa() is an RCU callback and therefore runs in softirq context,
> causing a kernel crash when the underlying AEAD implementation
> performs DMA unmapping during tfm destruction:
> 
>   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
> 
> Fix this by splitting free_rxsa() into two parts: the RCU callback
> now only schedules a work item, and the actual resource release
> (crypto_free_aead, free_percpu, kfree) is done in a workqueue
> handler running in process context.
> 
> Add a destroy_work field to struct macsec_rx_sa and initialize it
> in init_rx_sa().
> 
> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> ---
>  drivers/net/macsec.c | 13 +++++++++++--
>  include/net/macsec.h |  2 ++
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index f6cad0746a02..dabd3d2598ae 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -174,15 +174,23 @@ 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(work, struct macsec_rx_sa,
> +					       destroy_work);
>  
>  	crypto_free_aead(sa->key.tfm);
>  	free_percpu(sa->stats);
>  	kfree(sa);
>  }
>  
> +static void free_rxsa(struct rcu_head *head)
> +{
> +	struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu);
> +
> +	schedule_work(&sa->destroy_work);

rcu_work is what you want.

Re: [PATCH] macsec: defer RX SA cleanup from RCU callback to workqueue

[Sabrina Dubroca]

2026-05-06, 18:01:07 +0800, alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
> 
> crypto_free_aead() can call vunmap() internally (e.g. via
> dma_free_attrs() in hardware crypto drivers like hisi_sec2), which
> must not be called from softirq context.

Ok.

> free_rxsa() is an RCU callback and therefore runs in softirq context,
> causing a kernel crash when the underlying AEAD implementation
> performs DMA unmapping during tfm destruction:
> 
>   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
> 
> Fix this by splitting free_rxsa() into two parts: the RCU callback
> now only schedules a work item, and the actual resource release
> (crypto_free_aead, free_percpu, kfree) is done in a workqueue
> handler running in process context.
> 
> Add a destroy_work field to struct macsec_rx_sa and initialize it
> in init_rx_sa().

TXSAs go through exactly the same process (destruct via RCU and call
crypto_free_aead). I guess they would need exactly the same fix.


> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>

Missing a Fixes tag (most likely c09440f7dcb3 ("macsec: introduce IEEE
802.1AE driver")).

>  drivers/net/macsec.c | 13 +++++++++++--
>  include/net/macsec.h |  2 ++
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index f6cad0746a02..dabd3d2598ae 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -174,15 +174,23 @@ 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(work, struct macsec_rx_sa,
> +					       destroy_work);
>  
>  	crypto_free_aead(sa->key.tfm);
>  	free_percpu(sa->stats);
>  	kfree(sa);
>  }
>  
> +static void free_rxsa(struct rcu_head *head)
> +{
> +	struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu);
> +
> +	schedule_work(&sa->destroy_work);
> +}

This is quite ugly. I'd prefer to change the call_rcu() in
macsec_rxsa_put() to the schedule_work(), and then add a
synchronize_rcu() (to replace the current call_rcu()'s effects) at the
start of free_rxsa_work().

In addition, you need to modify macsec_exit() so that it waits on the
free_rxsa_work() calls. Otherwise, if they happen after the module has
finished unloading, the kernel will crash. Currently there's an
rcu_barrier() that waits for free_rxsa() running as RCU callback, but
it won't wait for the new work.

-- 
Sabrina