Re: [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid

["MOESSBAUER, Felix" <felix.moessbauer@siemens.com>]

On Mon, 2024-09-16 at 03:07 -0600, Jens Axboe wrote:
> A recent commit ensured that SQPOLL cannot be setup with a CPU that
> isn't in the current tasks cpuset, but it also dropped testing
> whether
> the CPU is valid in the first place. Without that, if a task passes
> in
> a CPU value that is too high, the following KASAN splat can get
> triggered:
> 
> BUG: KASAN: stack-out-of-bounds in io_sq_offload_create+0x858/0xaa4
> Read of size 8 at addr ffff800089bc7b90 by task wq-aff.t/1391
> 
> CPU: 4 UID: 1000 PID: 1391 Comm: wq-aff.t Not tainted 6.11.0-rc7-
> 00227-g371c468f4db6 #7080
> Hardware name: linux,dummy-virt (DT)
> Call trace:
>  dump_backtrace.part.0+0xcc/0xe0
>  show_stack+0x14/0x1c
>  dump_stack_lvl+0x58/0x74
>  print_report+0x16c/0x4c8
>  kasan_report+0x9c/0xe4
>  __asan_report_load8_noabort+0x1c/0x24
>  io_sq_offload_create+0x858/0xaa4
>  io_uring_setup+0x1394/0x17c4
>  __arm64_sys_io_uring_setup+0x6c/0x180
>  invoke_syscall+0x6c/0x260
>  el0_svc_common.constprop.0+0x158/0x224
>  do_el0_svc+0x3c/0x5c
>  el0_svc+0x34/0x70
>  el0t_64_sync_handler+0x118/0x124
>  el0t_64_sync+0x168/0x16c
> 
> The buggy address belongs to stack of task wq-aff.t/1391
>  and is located at offset 48 in frame:
>  io_sq_offload_create+0x0/0xaa4
> 
> This frame has 1 object:
>  [32, 40) 'allowed_mask'
> 
> The buggy address belongs to the virtual mapping at
>  [ffff800089bc0000, ffff800089bc9000) created by:
>  kernel_clone+0x124/0x7e0
> 
> The buggy address belongs to the physical page:
> page: refcount:1 mapcount:0 mapping:0000000000000000
> index:0xffff0000d740af80 pfn:0x11740a
> memcg:ffff0000c2706f02
> flags: 0xbffe00000000000(node=0|zone=2|lastcpupid=0x1fff)
> raw: 0bffe00000000000 0000000000000000 dead000000000122
> 0000000000000000
> raw: ffff0000d740af80 0000000000000000 00000001ffffffff
> ffff0000c2706f02
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  ffff800089bc7a80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff800089bc7b00: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> > ffff800089bc7b80: 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>                          ^
>  ffff800089bc7c00: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>  ffff800089bc7c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f3
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes:
> https://lore.kernel.org/oe-lkp/202409161632.cbeeca0d-lkp@intel.com
> Fixes: f011c9cf04c0 ("io_uring/sqpoll: do not allow pinning outside
> of cpuset")
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> ---
> 
> diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c
> index 272df9d00f45..7adfcf6818ff 100644
> --- a/io_uring/sqpoll.c
> +++ b/io_uring/sqpoll.c
> @@ -465,6 +465,8 @@ __cold int io_sq_offload_create(struct
> io_ring_ctx *ctx,
>                         int cpu = p->sq_thread_cpu;
>  
>                         ret = -EINVAL;
> +                       if (cpu >= nr_cpu_ids || !cpu_online(cpu))
> +                               goto err_sqpoll;

Thanks for fixing. I'm just wondering if cpu_online is really needed,
as offline CPUs are in the mask as well, aren't they?

Anyways: Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>

Felix

>                         cpuset_cpus_allowed(current, &allowed_mask);
>                         if (!cpumask_test_cpu(cpu, &allowed_mask))
>                                 goto err_sqpoll;

-- 
Siemens AG, Technology
Linux Expert Center