netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback
@ 2025-10-01 11:16 Pavel Zhigulin
  2025-10-04  4:28 ` Zhu Yanjun
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Zhigulin @ 2025-10-01 11:16 UTC (permalink / raw)
  To: Ayush Sawal
  Cc: Pavel Zhigulin, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Leon Romanovsky, Steffen Klassert,
	Cosmin Ratiu, Zhu Yanjun, Harsh Jain, Atul Gupta, Herbert Xu,
	Ganesh Goudar, netdev, linux-kernel, lvc-project

In ch_ipsec_xfrm_add_state() there is not check of try_module_get
return value. It is very unlikely, but try_module_get() could return
false value, which could cause use-after-free error.

This fix adds checking the result of try_module_get call

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 6dad4e8ab3ec ("chcr: Add support for Inline IPSec")
Signed-off-by: Pavel Zhigulin <Pavel.Zhigulin@kaspersky.com>
---
 .../net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
index ecd9a0bd5e18..3a5277630afa 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
@@ -35,6 +35,8 @@
  *	Atul Gupta (atul.gupta@chelsio.com)
  */

+#include "asm-generic/errno-base.h"
+#include "linux/compiler.h"
 #define pr_fmt(fmt) "ch_ipsec: " fmt

 #include <linux/kernel.h>
@@ -301,7 +303,8 @@ static int ch_ipsec_xfrm_add_state(struct net_device *dev,
 		sa_entry->esn = 1;
 	ch_ipsec_setkey(x, sa_entry);
 	x->xso.offload_handle = (unsigned long)sa_entry;
-	try_module_get(THIS_MODULE);
+	if (unlikely(!try_module_get(THIS_MODULE)))
+		res = -ENODEV;
 out:
 	return res;
 }
--
2.43.0


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

* Re: [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback
  2025-10-01 11:16 [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback Pavel Zhigulin
@ 2025-10-04  4:28 ` Zhu Yanjun
  2025-10-06 18:03   ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Zhu Yanjun @ 2025-10-04  4:28 UTC (permalink / raw)
  To: Pavel Zhigulin, Ayush Sawal
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Leon Romanovsky, Steffen Klassert, Cosmin Ratiu,
	Harsh Jain, Atul Gupta, Herbert Xu, Ganesh Goudar, netdev,
	linux-kernel, lvc-project


在 2025/10/1 4:16, Pavel Zhigulin 写道:
> In ch_ipsec_xfrm_add_state() there is not check of try_module_get
> return value. It is very unlikely, but try_module_get() could return
> false value, which could cause use-after-free error.
>
> This fix adds checking the result of try_module_get call
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 6dad4e8ab3ec ("chcr: Add support for Inline IPSec")
> Signed-off-by: Pavel Zhigulin <Pavel.Zhigulin@kaspersky.com>
> ---
>   .../net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
> index ecd9a0bd5e18..3a5277630afa 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c
> @@ -35,6 +35,8 @@
>    *	Atul Gupta (atul.gupta@chelsio.com)
>    */
>
> +#include "asm-generic/errno-base.h"
> +#include "linux/compiler.h"
>   #define pr_fmt(fmt) "ch_ipsec: " fmt
>
>   #include <linux/kernel.h>
> @@ -301,7 +303,8 @@ static int ch_ipsec_xfrm_add_state(struct net_device *dev,
>   		sa_entry->esn = 1;
>   	ch_ipsec_setkey(x, sa_entry);
>   	x->xso.offload_handle = (unsigned long)sa_entry;
> -	try_module_get(THIS_MODULE);
> +	if (unlikely(!try_module_get(THIS_MODULE)))

The function try_module_get() fails (returns false) only if the module 
is in the process of being

unloaded (i.e., module_refcount can’t be increased because the state is 
GOING

or UNFORMED). Otherwise, it succeeds and increments the refcount.

When the function ch_ipsec_xfrm_add_state is called, the kernel module 
cannot be in the GOING

or UNFORMED state. In other words, within this function, it is not 
possible for try_module_get to fail.

I am not sure if this check is strictly necessary, but as a defensive 
programming measure, it still makes sense.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Zhu Yanjun


> +		res = -ENODEV;
>   out:
>   	return res;
>   }
> --
> 2.43.0
>
-- 
Best Regards,
Yanjun.Zhu


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

* Re: [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback
  2025-10-04  4:28 ` Zhu Yanjun
@ 2025-10-06 18:03   ` Jakub Kicinski
  2025-10-06 18:27     ` Yanjun.Zhu
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2025-10-06 18:03 UTC (permalink / raw)
  To: Zhu Yanjun
  Cc: Pavel Zhigulin, Ayush Sawal, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Leon Romanovsky, Steffen Klassert,
	Cosmin Ratiu, Harsh Jain, Atul Gupta, Herbert Xu, Ganesh Goudar,
	netdev, linux-kernel, lvc-project

On Fri, 3 Oct 2025 21:28:51 -0700 Zhu Yanjun wrote:
> When the function ch_ipsec_xfrm_add_state is called, the kernel module 
> cannot be in the GOING or UNFORMED state.

That was my intuition as well, but on a quick look module state is set
to GOING before ->exit() is called. So this function can in fact fail
to acquire a reference.

Could you share your exact analysis?

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

* Re: [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback
  2025-10-06 18:03   ` Jakub Kicinski
@ 2025-10-06 18:27     ` Yanjun.Zhu
  0 siblings, 0 replies; 4+ messages in thread
From: Yanjun.Zhu @ 2025-10-06 18:27 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Pavel Zhigulin, Ayush Sawal, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Leon Romanovsky, Steffen Klassert,
	Cosmin Ratiu, Harsh Jain, Atul Gupta, Herbert Xu, Ganesh Goudar,
	netdev, linux-kernel, lvc-project


On 10/6/25 11:03 AM, Jakub Kicinski wrote:
> On Fri, 3 Oct 2025 21:28:51 -0700 Zhu Yanjun wrote:
>> When the function ch_ipsec_xfrm_add_state is called, the kernel module
>> cannot be in the GOING or UNFORMED state.
> That was my intuition as well, but on a quick look module state is set
> to GOING before ->exit() is called. So this function can in fact fail
> to acquire a reference.
>
> Could you share your exact analysis?

I delved into this function ch_ipsec_xfrm_add_state.

Yes — your understanding is correct:

When a module begins unloading, the kernel sets its state to GOING 
before invoking its ->exit() method.

Any concurrent call to try_module_get() will fail after this point.

So try_module_get() can return false, even though it’s extremely rare.

Ignoring that failure means continuing to use data that may already be 
in teardown, creating a use-after-free hazard.

This commit properly closes that race window.

Yanjun.Zhu


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

end of thread, other threads:[~2025-10-06 18:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-01 11:16 [PATCH] net: fix potential use-after-free in ch_ipsec_xfrm_add_state() callback Pavel Zhigulin
2025-10-04  4:28 ` Zhu Yanjun
2025-10-06 18:03   ` Jakub Kicinski
2025-10-06 18:27     ` Yanjun.Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).