[RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[Xiangyu Chen]

From: Qiao Ma <mqaio@linux.alibaba.com>

[ Upstream commit 373b9338c9722a368925d83bc622c596896b328e ]

Uprobe needs to fetch args into a percpu buffer, and then copy to ring
buffer to avoid non-atomic context problem.

Sometimes user-space strings, arrays can be very large, but the size of
percpu buffer is only page size. And store_trace_args() won't check
whether these data exceeds a single page or not, caused out-of-bounds
memory access.

It could be reproduced by following steps:
1. build kernel with CONFIG_KASAN enabled
2. save follow program as test.c

```
\#include <stdio.h>
\#include <stdlib.h>
\#include <string.h>

// If string length large than MAX_STRING_SIZE, the fetch_store_strlen()
// will return 0, cause __get_data_size() return shorter size, and
// store_trace_args() will not trigger out-of-bounds access.
// So make string length less than 4096.
\#define STRLEN 4093

void generate_string(char *str, int n)
{
    int i;
    for (i = 0; i < n; ++i)
    {
        char c = i % 26 + 'a';
        str[i] = c;
    }
    str[n-1] = '\0';
}

void print_string(char *str)
{
    printf("%s\n", str);
}

int main()
{
    char tmp[STRLEN];

    generate_string(tmp, STRLEN);
    print_string(tmp);

    return 0;
}
```
3. compile program
`gcc -o test test.c`

4. get the offset of `print_string()`
```
objdump -t test | grep -w print_string
0000000000401199 g     F .text  000000000000001b              print_string
```

5. configure uprobe with offset 0x1199
```
off=0x1199

cd /sys/kernel/debug/tracing/
echo "p /root/test:${off} arg1=+0(%di):ustring arg2=\$comm arg3=+0(%di):ustring"
 > uprobe_events
echo 1 > events/uprobes/enable
echo 1 > tracing_on
```

6. run `test`, and kasan will report error.
==================================================================
BUG: KASAN: use-after-free in strncpy_from_user+0x1d6/0x1f0
Write of size 8 at addr ffff88812311c004 by task test/499CPU: 0 UID: 0 PID: 499 Comm: test Not tainted 6.12.0-rc3+ #18
Hardware name: Red Hat KVM, BIOS 1.16.0-4.al8 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x55/0x70
 print_address_description.constprop.0+0x27/0x310
 kasan_report+0x10f/0x120
 ? strncpy_from_user+0x1d6/0x1f0
 strncpy_from_user+0x1d6/0x1f0
 ? rmqueue.constprop.0+0x70d/0x2ad0
 process_fetch_insn+0xb26/0x1470
 ? __pfx_process_fetch_insn+0x10/0x10
 ? _raw_spin_lock+0x85/0xe0
 ? __pfx__raw_spin_lock+0x10/0x10
 ? __pte_offset_map+0x1f/0x2d0
 ? unwind_next_frame+0xc5f/0x1f80
 ? arch_stack_walk+0x68/0xf0
 ? is_bpf_text_address+0x23/0x30
 ? kernel_text_address.part.0+0xbb/0xd0
 ? __kernel_text_address+0x66/0xb0
 ? unwind_get_return_address+0x5e/0xa0
 ? __pfx_stack_trace_consume_entry+0x10/0x10
 ? arch_stack_walk+0xa2/0xf0
 ? _raw_spin_lock_irqsave+0x8b/0xf0
 ? __pfx__raw_spin_lock_irqsave+0x10/0x10
 ? depot_alloc_stack+0x4c/0x1f0
 ? _raw_spin_unlock_irqrestore+0xe/0x30
 ? stack_depot_save_flags+0x35d/0x4f0
 ? kasan_save_stack+0x34/0x50
 ? kasan_save_stack+0x24/0x50
 ? mutex_lock+0x91/0xe0
 ? __pfx_mutex_lock+0x10/0x10
 prepare_uprobe_buffer.part.0+0x2cd/0x500
 uprobe_dispatcher+0x2c3/0x6a0
 ? __pfx_uprobe_dispatcher+0x10/0x10
 ? __kasan_slab_alloc+0x4d/0x90
 handler_chain+0xdd/0x3e0
 handle_swbp+0x26e/0x3d0
 ? __pfx_handle_swbp+0x10/0x10
 ? uprobe_pre_sstep_notifier+0x151/0x1b0
 irqentry_exit_to_user_mode+0xe2/0x1b0
 asm_exc_int3+0x39/0x40
RIP: 0033:0x401199
Code: 01 c2 0f b6 45 fb 88 02 83 45 fc 01 8b 45 fc 3b 45 e4 7c b7 8b 45 e4 48 98 48 8d 50 ff 48 8b 45 e8 48 01 d0 ce
RSP: 002b:00007ffdf00576a8 EFLAGS: 00000206
RAX: 00007ffdf00576b0 RBX: 0000000000000000 RCX: 0000000000000ff2
RDX: 0000000000000ffc RSI: 0000000000000ffd RDI: 00007ffdf00576b0
RBP: 00007ffdf00586b0 R08: 00007feb2f9c0d20 R09: 00007feb2f9c0d20
R10: 0000000000000001 R11: 0000000000000202 R12: 0000000000401040
R13: 00007ffdf0058780 R14: 0000000000000000 R15: 0000000000000000
 </TASK>

This commit enforces the buffer's maxlen less than a page-size to avoid
store_trace_args() out-of-memory access.

Link: https://lore.kernel.org/all/20241015060148.1108331-1-mqaio@linux.alibaba.com/

Fixes: dcad1a204f72 ("tracing/uprobes: Fetch args before reserving a ring buffer")
Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
[ the prepare_uprobe_buffer() function was introduced by commit 3eaea21b4d27 
("uprobes: encapsulate preparation of uprobe args buffer"). So the fix on 5.10/15
need to modify the code in uprobe_dispatcher() and uretprobe_dispatcher() before
calling the store_trace_args. ]
Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Try to backport the fix to 5.10/5.15.
Verified the build test.
Verified the changes with test code in comment, with the fix, the KASAN crash won't
happen anymore.
---
 kernel/trace/trace_uprobe.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 720b46b34ab9..9f8c95b9b30d 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -858,6 +858,7 @@ struct uprobe_cpu_buffer {
 };
 static struct uprobe_cpu_buffer __percpu *uprobe_cpu_buffer;
 static int uprobe_buffer_refcnt;
+#define MAX_UCB_BUFFER_SIZE PAGE_SIZE
 
 static int uprobe_buffer_init(void)
 {
@@ -958,9 +959,6 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
 
 	WARN_ON(call != trace_file->event_call);
 
-	if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
-		return;
-
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
 
@@ -1503,6 +1501,10 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
 	esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
 
 	ucb = uprobe_buffer_get();
+
+	if (WARN_ON_ONCE(tu->tp.size + dsize > MAX_UCB_BUFFER_SIZE))
+		dsize = MAX_UCB_BUFFER_SIZE - tu->tp.size;
+
 	store_trace_args(ucb->buf, &tu->tp, regs, esize, dsize);
 
 	if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
@@ -1538,6 +1540,10 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
 	esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
 
 	ucb = uprobe_buffer_get();
+
+	if (WARN_ON_ONCE(tu->tp.size + dsize > MAX_UCB_BUFFER_SIZE))
+		dsize = MAX_UCB_BUFFER_SIZE - tu->tp.size;
+
 	store_trace_args(ucb->buf, &tu->tp, regs, esize, dsize);
 
 	if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
-- 
2.25.1

Re: [RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[Greg KH]

On Mon, Mar 17, 2025 at 02:54:29PM +0800, Xiangyu Chen wrote:
> From: Qiao Ma <mqaio@linux.alibaba.com>
> 
> [ Upstream commit 373b9338c9722a368925d83bc622c596896b328e ]

<snip>

Why is this an RFC?  What needs to be done to make it "real" and ready
for you to submit it for actual inclusion?

thanks,

greg k-h

Re: [RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[Xiangyu Chen]

On 3/17/25 14:57, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Mon, Mar 17, 2025 at 02:54:29PM +0800, Xiangyu Chen wrote:
>> From: Qiao Ma <mqaio@linux.alibaba.com>
>>
>> [ Upstream commit 373b9338c9722a368925d83bc622c596896b328e ]
Hi Greg,
> <snip>
>
> Why is this an RFC?  What needs to be done to make it "real" and ready
> for you to submit it for actual inclusion?

We try to backport the fix to 5.15/5.10, but some logic functions are 
different, the prepare_uprobe_buffer() in original

commit is not exists on 5.15/5.10,  we moved the fix to 
uprobe_dispatcher() and uretprobe_dispatcher().

It has been tested in our local environment, the issue was fixed, but 
due to it different from original commit,

this might still need to author help to review, so I added a RFC label.


Thanks,


Br,

Xiangyu

>
> thanks,
>
> greg k-h

Re: [RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[Greg KH]

On Mon, Mar 17, 2025 at 03:09:50PM +0800, Xiangyu Chen wrote:
> 
> On 3/17/25 14:57, Greg KH wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > On Mon, Mar 17, 2025 at 02:54:29PM +0800, Xiangyu Chen wrote:
> > > From: Qiao Ma <mqaio@linux.alibaba.com>
> > > 
> > > [ Upstream commit 373b9338c9722a368925d83bc622c596896b328e ]
> Hi Greg,
> > <snip>
> > 
> > Why is this an RFC?  What needs to be done to make it "real" and ready
> > for you to submit it for actual inclusion?
> 
> We try to backport the fix to 5.15/5.10, but some logic functions are
> different, the prepare_uprobe_buffer() in original
> 
> commit is not exists on 5.15/5.10,  we moved the fix to uprobe_dispatcher()
> and uretprobe_dispatcher().
> 
> It has been tested in our local environment, the issue was fixed, but due to
> it different from original commit,
> 
> this might still need to author help to review, so I added a RFC label.

If you want people to do reviews / work / etc, then you explicitly need
to ask for that.  Otherwise we all have no idea what problems you have
with this change, nor what you expect for anyone else to do.

First off, why do you think this needs to be backported here at all?  Do
that research and work first, and figure it out with your own testing
and evaluation before asking others to do any work for you.

good luck!

greg k-h

Re: [RFC PATCH 5.15/5.10] uprobe: avoid out-of-bounds memory access of fetching args

[Xiangyu Chen]

On 3/17/25 15:11, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Mon, Mar 17, 2025 at 03:09:50PM +0800, Xiangyu Chen wrote:
>> On 3/17/25 14:57, Greg KH wrote:
>>> CAUTION: This email comes from a non Wind River email account!
>>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>> On Mon, Mar 17, 2025 at 02:54:29PM +0800, Xiangyu Chen wrote:
>>>> From: Qiao Ma <mqaio@linux.alibaba.com>
>>>>
>>>> [ Upstream commit 373b9338c9722a368925d83bc622c596896b328e ]
>> Hi Greg,
>>> <snip>
>>>
>>> Why is this an RFC?  What needs to be done to make it "real" and ready
>>> for you to submit it for actual inclusion?
>> We try to backport the fix to 5.15/5.10, but some logic functions are
>> different, the prepare_uprobe_buffer() in original
>>
>> commit is not exists on 5.15/5.10,  we moved the fix to uprobe_dispatcher()
>> and uretprobe_dispatcher().
>>
>> It has been tested in our local environment, the issue was fixed, but due to
>> it different from original commit,
>>
>> this might still need to author help to review, so I added a RFC label.
Hi Greg,
> If you want people to do reviews / work / etc, then you explicitly need
> to ask for that.  Otherwise we all have no idea what problems you have
> with this change, nor what you expect for anyone else to do.
>
> First off, why do you think this needs to be backported here at all?  Do
> that research and work first, and figure it out with your own testing
> and evaluation before asking others to do any work for you.
>
> good luck!

Thanks for your advice, I'll pay attention next time, and better use a 
cover-letter to describe the detail information to others instead of RFC 
label .


We backport this patch due to a high risky security issue 
(CVE-2024-50067),  the 6.1+ already fixed, but 5.10/15 still exists. 
Since the uprobe is a common feature in kernel, so it should be fixed.


I read the fix and context commits, on 5.15/10, the original fix in 
prepare_uprobe_buffer()'s logic can be apply on uretprobe_dispatcher() 
uprobe_dispatcher().  Based on that, I have verified it on a local setup 
by manual,  without the fix, 5.10/5.15 can be reproduced the KASAN error 
by test code in commit comment, after applying the patch, the KASAN 
error won't happen anymore.


It was verified on my local setup, but the fix on 5.10/15 is fully 
different from original commit,  so in order to make sure it can be 
"clean" fixed, this need to author help to review if anything was missing.


Thanks,


Br,

Xiangyu

> greg k-h
>