linux-kernel - Re: [PATCH] bitmap-str: Limit the size of ulen in bitmap_parselist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aF1a0w2scIR8zRnQ@yury>
Date: Thu, 26 Jun 2025 10:36:03 -0400
From: Yury Norov <yury.norov@...il.com>
To: Cheng Yu <serein.chengyu@...wei.com>
Cc: linux@...musvillemoes.dk, akpm@...ux-foundation.org, travis@....com,
	linux-kernel@...r.kernel.org, zhangqiao22@...wei.com,
	tanghui20@...wei.com, chenweilong@...wei.com,
	judy.chenhui@...wei.com, chenjun102@...wei.com
Subject: Re: [PATCH] bitmap-str: Limit the size of ulen in
 bitmap_parselist_user()

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@...wei.com>
> Signed-off-by: Wu Guanghao <wuguanghao3@...wei.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