[PATCH] uprobes: Fix NULL pointer dereference in hprobe_expire()

[PATCH] uprobes: Fix NULL pointer dereference in hprobe_expire()

[Breno Leitao]

Forking a task that has a pending uretprobe can oops the kernel with a
NULL pointer dereference in the clone() path:

  BUG: kernel NULL pointer dereference, address: 0000000000000018
  Oops: 0002 [#1] SMP NOPTI
  RIP: 0010:hprobe_expire
  CR2: 0000000000000018
  Call Trace:
   uprobe_copy_process
   copy_process
   kernel_clone
   __x64_sys_clone
   do_syscall_64
   entry_SYSCALL_64_after_hwframe

This was found on real hosts on Meta fleet.

I've got the impression that this is what is happening:

  CPU 1                          CPU 2 (traced task)
  -----                          -------------------
                                 hit uprobe, prepare_uretprobe():
                                   hprobe LEASED, refcount >= 1
  uprobe_unregister()
    put_uprobe(): refcount -> 0
                                 fork() -> dup_utask()
                                   hprobe_expire(hprobe, true)
                                     try_get_uprobe() -> NULL
                                     get_uprobe(NULL)   <-- Oops

Only take the extra reference when the uprobe is non-NULL; a NULL means
it is gone and is the correct value to return.

Fixes: dd1a7567784e ("uprobes: SRCU-protect uretprobe lifetime (with timeout)")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 07f69dd3093d5..950108f92079d 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -832,7 +832,7 @@ static struct uprobe *hprobe_expire(struct hprobe *hprobe, bool get)
 		if (try_cmpxchg(&hprobe->state, &hstate, uprobe ? HPROBE_STABLE : HPROBE_GONE)) {
 			/* We won the race, we are the ones to unlock SRCU */
 			__srcu_read_unlock(&uretprobes_srcu, hprobe->srcu_idx);
-			return get ? get_uprobe(uprobe) : uprobe;
+			return get && uprobe ? get_uprobe(uprobe) : uprobe;
 		}
 
 		/*

---
base-commit: 3652b49adac266a3d27cb41cdfdb7d8790fc3633
change-id: 20260729-uprobe-b142b0863572

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] uprobes: Fix NULL pointer dereference in hprobe_expire()

[Oleg Nesterov]

On 07/29, Breno Leitao wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -832,7 +832,7 @@ static struct uprobe *hprobe_expire(struct hprobe *hprobe, bool get)
>  		if (try_cmpxchg(&hprobe->state, &hstate, uprobe ? HPROBE_STABLE : HPROBE_GONE)) {
>  			/* We won the race, we are the ones to unlock SRCU */
>  			__srcu_read_unlock(&uretprobes_srcu, hprobe->srcu_idx);
> -			return get ? get_uprobe(uprobe) : uprobe;
> +			return get && uprobe ? get_uprobe(uprobe) : uprobe;

Well, looks "obviously correct". At least the current code is obviously
wrong, it even checks uprobe != NULL 3 lines above.

Andrii ?

Acked-by: Oleg Nesterov <oleg@redhat.com>