Re: [PATCH] mm: memory: fix shift-out-of-bounds in fault_around_bytes_set

[Lorenzo Stoakes]

On Sat, Mar 02, 2024 at 02:43:12PM +0800, Kefeng Wang wrote:
> The rounddown_pow_of_two(0) is undefined, so val = 0 is not allowed in
> the fault_around_bytes_set(), and leads to shift-out-of-bounds,
>
> UBSAN: shift-out-of-bounds in include/linux/log2.h:67:13
> shift exponent 4294967295 is too large for 64-bit type 'long unsigned int'
> CPU: 7 PID: 107 Comm: sh Not tainted 6.8.0-rc6-next-20240301 #294
> Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
> Call trace:
>  dump_backtrace+0x94/0xec
>  show_stack+0x18/0x24
>  dump_stack_lvl+0x78/0x90
>  dump_stack+0x18/0x24
>  ubsan_epilogue+0x10/0x44
>  __ubsan_handle_shift_out_of_bounds+0x98/0x134
>  fault_around_bytes_set+0xa4/0xb0
>  simple_attr_write_xsigned.isra.0+0xe4/0x1ac
>  simple_attr_write+0x18/0x24
>  debugfs_attr_write+0x4c/0x98
>  vfs_write+0xd0/0x4b0
>  ksys_write+0x6c/0xfc
>  __arm64_sys_write+0x1c/0x28
>  invoke_syscall+0x44/0x104
>  el0_svc_common.constprop.0+0x40/0xe0
>  do_el0_svc+0x1c/0x28
>  el0_svc+0x34/0xdc
>  el0t_64_sync_handler+0xc0/0xc4
>  el0t_64_sync+0x190/0x194
> ---[ end trace ]---
>
> Fix it by setting the minimum val to PAGE_SIZE.
>
> Reported-by: Yue Sun <samsun1006219@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/CAEkJfYPim6DQqW1GqCiHLdh2-eweqk1fGyXqs3JM+8e1qGge8w@xxxxxxxxxxxxxx/
> Fixes: 53d36a56d8c4 ("mm: prefer fault_around_pages to fault_around_bytes")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> ---
>  mm/memory.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index abd4f33d62c9..e17669d4f72f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4776,7 +4776,8 @@ static int fault_around_bytes_set(void *data, u64 val)
>  	 * The minimum value is 1 page, however this results in no fault-around
>  	 * at all. See should_fault_around().
>  	 */
> -	fault_around_pages = max(rounddown_pow_of_two(val) >> PAGE_SHIFT, 1UL);
> +	val = max(val, PAGE_SIZE);
> +	fault_around_pages = rounddown_pow_of_two(val) >> PAGE_SHIFT;
>
>  	return 0;
>  }
> --
> 2.27.0
>

Thanks, this was an oversight on my part, this fix looks good:

Reviewed-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>