mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
@ 2026-07-17  8:26 Mingyu Wang
  2026-08-04  6:18 ` Mingyu Wang
  2026-08-04  8:42 ` Petr Pavlu
  0 siblings, 2 replies; 6+ messages in thread
From: Mingyu Wang @ 2026-07-17  8:26 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Aaron Tomlin, Johan Hovold, linux-modules, linux-kernel,
	Mingyu Wang, stable

During concurrent module loading (e.g., triggered by syzkaller), the
idempotent module loading mechanism uses a local stack variable
(`struct idempotent idem`) to track the state of waiters.

If a task executing idempotent_init_module() is abruptly terminated
(e.g., killed by a fatal signal or otherwise completely exits before
reaching the list cleanup paths) after adding its node to the global
`idem_hash` list, its kernel stack is prematurely freed and reclaimed.

However, the stack-allocated node remains linked in the list. Subsequent
module loading attempts that traverse the list will dereference this
stale stack pointer, leading to KASAN slab-out-of-bounds reads and
General Protection Faults (GPF):

  BUG: KASAN: slab-out-of-bounds in idempotent_init_module+0x54a/0x620
  Read of size 8 at addr ffff888106367df8 by task modprobe/433
  ...
  The buggy address belongs to the object at ffff8881063676c0
   which belongs to the cache shmem_inode_cache of size 1392
  ...
  Oops: general protection fault, probably for non-canonical address

Fix this by dynamically allocating `struct idempotent` on the heap
via `kmalloc_obj()`. This decouples the list node's lifespan from the
process stack, ensuring that even if the task is abruptly terminated,
the global list safely points to valid heap memory until properly
unlinked, preventing memory corruption.

Fixes: 9b9879fc0327 ("modules: catch concurrent module loads, treat them as idempotent")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 kernel/module/main.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..39f05ac4b1a1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3779,21 +3779,29 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 
 static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
 {
-	struct idempotent idem;
+	struct idempotent *idem;
+	int ret;
 
 	if (!(f->f_mode & FMODE_READ))
 		return -EBADF;
 
+	idem = kmalloc_obj(*idem, GFP_KERNEL);
+	if (!idem)
+		return -ENOMEM;
+
 	/* Are we the winners of the race and get to do this? */
-	if (!idempotent(&idem, file_inode(f))) {
-		int ret = init_module_from_file(f, uargs, flags);
-		return idempotent_complete(&idem, ret);
+	if (!idempotent(idem, file_inode(f))) {
+		ret = init_module_from_file(f, uargs, flags);
+		ret = idempotent_complete(idem, ret);
+	} else {
+		/*
+		 * Somebody else won the race and is loading the module.
+		 */
+		ret = idempotent_wait_for_completion(idem);
 	}
 
-	/*
-	 * Somebody else won the race and is loading the module.
-	 */
-	return idempotent_wait_for_completion(&idem);
+	kfree(idem);
+	return ret;
 }
 
 SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
-- 
2.34.1


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

* Re: [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
  2026-07-17  8:26 [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation Mingyu Wang
@ 2026-08-04  6:18 ` Mingyu Wang
  2026-08-04  8:42 ` Petr Pavlu
  1 sibling, 0 replies; 6+ messages in thread
From: Mingyu Wang @ 2026-08-04  6:18 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Aaron Tomlin, Johan Hovold, linux-modules, linux-kernel, stable

Hi Luis, Petr, and others,

Gentle ping on this patch.
Please let me know if there are any updates or if further modifications 
are needed from my side.

Thanks,
Mingyu


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

* Re: [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
  2026-07-17  8:26 [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation Mingyu Wang
  2026-08-04  6:18 ` Mingyu Wang
@ 2026-08-04  8:42 ` Petr Pavlu
  2026-08-04 10:41   ` Mingyu Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Petr Pavlu @ 2026-08-04  8:42 UTC (permalink / raw)
  To: Mingyu Wang
  Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
	Johan Hovold, linux-modules, linux-kernel, stable

On 7/17/26 10:26 AM, Mingyu Wang wrote:
> During concurrent module loading (e.g., triggered by syzkaller), the
> idempotent module loading mechanism uses a local stack variable
> (`struct idempotent idem`) to track the state of waiters.
> 
> If a task executing idempotent_init_module() is abruptly terminated
> (e.g., killed by a fatal signal or otherwise completely exits before
> reaching the list cleanup paths) after adding its node to the global
> `idem_hash` list, its kernel stack is prematurely freed and reclaimed.

I'm confused by this description and I'm not sure what problem the patch
is trying to solve. A task is not normally terminated in the middle of
a syscall. This could happen only if it hits an oops. However, that
means there is an earlier bug and the module loader isn't expected to be
resilient to a post-oops state.

Could you clarify the scenario that you ran into?

-- 
Thanks,
Petr

> 
> However, the stack-allocated node remains linked in the list. Subsequent
> module loading attempts that traverse the list will dereference this
> stale stack pointer, leading to KASAN slab-out-of-bounds reads and
> General Protection Faults (GPF):
> 
>   BUG: KASAN: slab-out-of-bounds in idempotent_init_module+0x54a/0x620
>   Read of size 8 at addr ffff888106367df8 by task modprobe/433
>   ...
>   The buggy address belongs to the object at ffff8881063676c0
>    which belongs to the cache shmem_inode_cache of size 1392
>   ...
>   Oops: general protection fault, probably for non-canonical address
> 
> Fix this by dynamically allocating `struct idempotent` on the heap
> via `kmalloc_obj()`. This decouples the list node's lifespan from the
> process stack, ensuring that even if the task is abruptly terminated,
> the global list safely points to valid heap memory until properly
> unlinked, preventing memory corruption.
> 
> Fixes: 9b9879fc0327 ("modules: catch concurrent module loads, treat them as idempotent")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
>  kernel/module/main.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a605..39f05ac4b1a1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3779,21 +3779,29 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>  
>  static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
>  {
> -	struct idempotent idem;
> +	struct idempotent *idem;
> +	int ret;
>  
>  	if (!(f->f_mode & FMODE_READ))
>  		return -EBADF;
>  
> +	idem = kmalloc_obj(*idem, GFP_KERNEL);
> +	if (!idem)
> +		return -ENOMEM;
> +
>  	/* Are we the winners of the race and get to do this? */
> -	if (!idempotent(&idem, file_inode(f))) {
> -		int ret = init_module_from_file(f, uargs, flags);
> -		return idempotent_complete(&idem, ret);
> +	if (!idempotent(idem, file_inode(f))) {
> +		ret = init_module_from_file(f, uargs, flags);
> +		ret = idempotent_complete(idem, ret);
> +	} else {
> +		/*
> +		 * Somebody else won the race and is loading the module.
> +		 */
> +		ret = idempotent_wait_for_completion(idem);
>  	}
>  
> -	/*
> -	 * Somebody else won the race and is loading the module.
> -	 */
> -	return idempotent_wait_for_completion(&idem);
> +	kfree(idem);
> +	return ret;
>  }
>  
>  SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)

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

* Re: [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
  2026-08-04  8:42 ` Petr Pavlu
@ 2026-08-04 10:41   ` Mingyu Wang
  2026-08-05 14:47     ` Petr Pavlu
  0 siblings, 1 reply; 6+ messages in thread
From: Mingyu Wang @ 2026-08-04 10:41 UTC (permalink / raw)
  To: Petr Pavlu
  Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
	Johan Hovold, linux-modules, linux-kernel, stable

Hi Petr,

> I'm confused by this description and I'm not sure what problem the patch
> is trying to solve. A task is not normally terminated in the middle of
> a syscall. This could happen only if it hits an oops. However, that
> means there is an earlier bug and the module loader isn't expected to be
> resilient to a post-oops state.
>
> Could you clarify the scenario that you ran into?

Thank you for the review. You are entirely correct; the task did not
terminate during a normal syscall, but was indeed killed by a prior
oops.

Here is the exact scenario discovered during driver fuzzing by our
automated virtual device modeling tool, DevGen:

1. A `modprobe` task (PID 433) concurrently loaded a faulty driver
    (the `atlantic` net driver in our trace).
2. It passed the idempotent() check and added its stack-allocated
    `struct idempotent` node to the global `idem_hash` list.
3. During init_module_from_file(), the `atlantic` driver hit a fatal
    oops in its hardware init path (aq_hw_write_reg+0x39/0x50).
4. The kernel initiated oops recovery, terminating the `modprobe` task
    (exited with preempt_count 1) and freeing its kernel stack.
5. The oops recovery path does not clean up the `idem_hash` list,
    leaving a dangling pointer to the freed stack in the global list.
6. A subsequent module loading attempt traversed `idem_hash`,
    dereferenced the stale stack pointer, and triggered a secondary
    KASAN use-after-free and GPF.

While the root cause of the initial crash lies within the specific
buggy driver, the current stack allocation allows an isolated driver
oops to corrupt a global synchronization primitive, preventing any
further module loading across the entire system.

By migrating this allocation to the heap via kmalloc_obj(), the patch
prevents the global memory corruption. We acknowledge this leaves a
leaked node in the hash list. Consequently, subsequent attempts to load
that *specific* oopsed module will hang, but the rest of the module
subsystem survives and functions normally.

Thanks,
Mingyu


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

* Re: [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
  2026-08-04 10:41   ` Mingyu Wang
@ 2026-08-05 14:47     ` Petr Pavlu
  2026-08-06  1:30       ` Mingyu Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Pavlu @ 2026-08-05 14:47 UTC (permalink / raw)
  To: Mingyu Wang
  Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
	Johan Hovold, linux-modules, linux-kernel, stable

On 8/4/26 12:41 PM, Mingyu Wang wrote:
> Hi Petr,
> 
>> I'm confused by this description and I'm not sure what problem the patch
>> is trying to solve. A task is not normally terminated in the middle of
>> a syscall. This could happen only if it hits an oops. However, that
>> means there is an earlier bug and the module loader isn't expected to be
>> resilient to a post-oops state.
>>
>> Could you clarify the scenario that you ran into?
> 
> Thank you for the review. You are entirely correct; the task did not
> terminate during a normal syscall, but was indeed killed by a prior
> oops.
> 
> Here is the exact scenario discovered during driver fuzzing by our
> automated virtual device modeling tool, DevGen:
> 
> 1. A `modprobe` task (PID 433) concurrently loaded a faulty driver
>    (the `atlantic` net driver in our trace).
> 2. It passed the idempotent() check and added its stack-allocated
>    `struct idempotent` node to the global `idem_hash` list.
> 3. During init_module_from_file(), the `atlantic` driver hit a fatal
>    oops in its hardware init path (aq_hw_write_reg+0x39/0x50).
> 4. The kernel initiated oops recovery, terminating the `modprobe` task
>    (exited with preempt_count 1) and freeing its kernel stack.
> 5. The oops recovery path does not clean up the `idem_hash` list,
>    leaving a dangling pointer to the freed stack in the global list.
> 6. A subsequent module loading attempt traversed `idem_hash`,
>    dereferenced the stale stack pointer, and triggered a secondary
>    KASAN use-after-free and GPF.

Thanks for explaining the scenario.

> 
> While the root cause of the initial crash lies within the specific
> buggy driver, the current stack allocation allows an isolated driver
> oops to corrupt a global synchronization primitive, preventing any
> further module loading across the entire system.

An oopsed task can leave behind various kernel structures in an
inconsistent state, such as unreleased locks, incorrect reference counts
and leaked objects. It is expected that the kernel may behave strangely
afterward and this can result in subsequent problems, such as deadlocks
or crashes.

Loading a module is a heavyweight operation that injects new code and
data into the kernel. If it ends in an oops, I think it is fair that no
additional modules can be inserted. I don't see that the module loader
is doing anything wrong or behaving differently from other kernel
components in this regard.

-- 
Cheers,
Petr

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

* Re: [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation
  2026-08-05 14:47     ` Petr Pavlu
@ 2026-08-06  1:30       ` Mingyu Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Mingyu Wang @ 2026-08-06  1:30 UTC (permalink / raw)
  To: Petr Pavlu
  Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
	Johan Hovold, linux-modules, linux-kernel, stable

> An oopsed task can leave behind various kernel structures in an
> inconsistent state, such as unreleased locks, incorrect reference counts
> and leaked objects. It is expected that the kernel may behave strangely
> afterward and this can result in subsequent problems, such as deadlocks
> or crashes.
>
> Loading a module is a heavyweight operation that injects new code and
> data into the kernel. If it ends in an oops, I think it is fair that no
> additional modules can be inserted. I don't see that the module loader
> is doing anything wrong or behaving differently from other kernel
> components in this regard.

Makes sense. Hardening the module loader against a post-oops state is

indeed unnecessary. I'll drop this and go after the driver bugs instead.

Thanks for the review.

Cheers,
Mingyu


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

end of thread, other threads:[~2026-08-06  1:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-17  8:26 [PATCH] module: fix UAF and GPF in idempotent_init_module via heap allocation Mingyu Wang
2026-08-04  6:18 ` Mingyu Wang
2026-08-04  8:42 ` Petr Pavlu
2026-08-04 10:41   ` Mingyu Wang
2026-08-05 14:47     ` Petr Pavlu
2026-08-06  1:30       ` Mingyu Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®