bitmap-str: Limit the size of ulen in bitmap_parselist_user()

[PATCH] bitmap-str: Limit the size of ulen in bitmap_parselist_user()

Posted by Cheng Yu 3 months, 2 weeks ago

Wu randomly sets the smp_affinity_list of the specified irq through a
code on the linux-5.10.y kernel:
    ================================
    ...
    memcpy((void*)0x20000080, "/proc/irq/18/smp_affinity_list", 30);
    res = syscall(__NR_openat, /*fd=*/0xffffffffffffff9cul,
                  /*file=*/0x20000080ul, /*flags=*/1ul, /*mode=*/0ul);
    r[0] = res;
    ...
    syscall(__NR_write, /*fd=*/r[0], /*arg=*/0x20002100ul,
            /*len=*/0xfffffdfeul);
    ...
    ================================

Warning will be triggered and the call trace is as follows:
[   69.732366] Call Trace:
[   69.732373]  ? show_trace_log_lvl+0x1c1/0x2d9
[   69.732374]  ? show_trace_log_lvl+0x1c1/0x2d9
[   69.732379]  ? kmalloc_order+0x28/0xf0
[   69.732389]  ? __alloc_pages_nodemask+0x287/0x300
[   69.732393]  ? __warn+0x80/0x100
[   69.732394]  ? __alloc_pages_nodemask+0x287/0x300
[   69.732397]  ? report_bug+0x9e/0xc0
[   69.732400]  ? handle_bug+0x41/0x90
[   69.732402]  ? exc_invalid_op+0x14/0x70
[   69.732403]  ? asm_exc_invalid_op+0x12/0x20
[   69.732405]  ? __alloc_pages_nodemask+0x287/0x300
[   69.732407]  ? bitmap_parselist+0x120/0x120
[   69.732409]  kmalloc_order+0x28/0xf0
[   69.732410]  kmalloc_order_trace+0x19/0x90
[   69.732414]  memdup_user_nul+0x22/0xa0
[   69.732415]  bitmap_parselist_user+0x35/0x80
[   69.732419]  write_irq_affinity.constprop.0.isra.0+0xb9/0x110
[   69.732422]  proc_reg_write+0x4e/0x90
[   69.732425]  vfs_write+0xbf/0x290
[   69.732426]  ksys_write+0x5f/0xe0
[   69.732428]  do_syscall_64+0x30/0x40
[   69.732430]  entry_SYSCALL_64_after_hwframe+0x67/0xd1

It is due to the lack of a check for the maximum value of ulen in
bitmap_parselist_user().

Fixes: 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq")
Signed-off-by: Cheng Yu <serein.chengyu@huawei.com>
Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
---
 lib/bitmap-str.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
index be745209507a..0b5d9ffe20c6 100644
--- a/lib/bitmap-str.c
+++ b/lib/bitmap-str.c
@@ -419,6 +419,9 @@ int bitmap_parselist_user(const char __user *ubuf,
 	char *buf;
 	int ret;
 
+	if (ulen > PAGE_SIZE)
+		return -ENOMEM;
+
 	buf = memdup_user_nul(ubuf, ulen);
 	if (IS_ERR(buf))
 		return PTR_ERR(buf);
-- 
2.25.1

Re: [PATCH] bitmap-str: Limit the size of ulen in bitmap_parselist_user()

Posted by Yury Norov 3 months, 1 week ago

On Thu, Jun 26, 2025 at 08:19:03PM +0800, Cheng Yu wrote:
> Wu randomly sets the smp_affinity_list of the specified irq through a

Is this Wu a type of fuzzer, or a person? We don't put persons names
in commit messages, other than in tags section.

> code on the linux-5.10.y kernel:
>     ================================
>     ...
>     memcpy((void*)0x20000080, "/proc/irq/18/smp_affinity_list", 30);
>     res = syscall(__NR_openat, /*fd=*/0xffffffffffffff9cul,
>                   /*file=*/0x20000080ul, /*flags=*/1ul, /*mode=*/0ul);
>     r[0] = res;
>     ...
>     syscall(__NR_write, /*fd=*/r[0], /*arg=*/0x20002100ul,
>             /*len=*/0xfffffdfeul);
>     ...
>     ================================
> 
> Warning will be triggered and the call trace is as follows:

So, can you show the warning itself?

> [   69.732366] Call Trace:
> [   69.732373]  ? show_trace_log_lvl+0x1c1/0x2d9
> [   69.732374]  ? show_trace_log_lvl+0x1c1/0x2d9
> [   69.732379]  ? kmalloc_order+0x28/0xf0
> [   69.732389]  ? __alloc_pages_nodemask+0x287/0x300
> [   69.732393]  ? __warn+0x80/0x100
> [   69.732394]  ? __alloc_pages_nodemask+0x287/0x300
> [   69.732397]  ? report_bug+0x9e/0xc0
> [   69.732400]  ? handle_bug+0x41/0x90
> [   69.732402]  ? exc_invalid_op+0x14/0x70
> [   69.732403]  ? asm_exc_invalid_op+0x12/0x20
> [   69.732405]  ? __alloc_pages_nodemask+0x287/0x300
> [   69.732407]  ? bitmap_parselist+0x120/0x120
> [   69.732409]  kmalloc_order+0x28/0xf0
> [   69.732410]  kmalloc_order_trace+0x19/0x90
> [   69.732414]  memdup_user_nul+0x22/0xa0
> [   69.732415]  bitmap_parselist_user+0x35/0x80
> [   69.732419]  write_irq_affinity.constprop.0.isra.0+0xb9/0x110
> [   69.732422]  proc_reg_write+0x4e/0x90
> [   69.732425]  vfs_write+0xbf/0x290
> [   69.732426]  ksys_write+0x5f/0xe0
> [   69.732428]  do_syscall_64+0x30/0x40
> [   69.732430]  entry_SYSCALL_64_after_hwframe+0x67/0xd1
> 
> It is due to the lack of a check for the maximum value of ulen in
> bitmap_parselist_user().
> 
> Fixes: 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq")
> Signed-off-by: Cheng Yu <serein.chengyu@huawei.com>
> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
> ---
>  lib/bitmap-str.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
> index be745209507a..0b5d9ffe20c6 100644
> --- a/lib/bitmap-str.c
> +++ b/lib/bitmap-str.c
> @@ -419,6 +419,9 @@ int bitmap_parselist_user(const char __user *ubuf,
>  	char *buf;
>  	int ret;
>  
> +	if (ulen > PAGE_SIZE)
> +		return -ENOMEM;

This is wrong. A valid cpulist can easily overflow the PAGE_SIZE
limit, see the CPULIST_FILE_MAX_BYTES() macro.

From general perspective, passing huge cpulists is not restricted now,
and what you're proposing here is the change of UAPI. I generally
agree that kernel should reject sizes like '(unsigned)-EINVAL', but the
change should be broadly discussed and documented.

Currently we don't reject huge sizes because users' scrips may
concatenate cpulists from different sources, and we theoretically may
end up with something like [0-1,3-4] x 1000 times, and it's a valid
pattern.

>  	buf = memdup_user_nul(ubuf, ulen);
>  	if (IS_ERR(buf))
>  		return PTR_ERR(buf);
> -- 
> 2.25.1