* [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid
@ 2024-09-16 9:07 Jens Axboe
2024-09-16 9:11 ` MOESSBAUER, Felix
0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2024-09-16 9:07 UTC (permalink / raw)
To: io-uring; +Cc: Felix Moessbauer
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 <[email protected]>
Closes: https://lore.kernel.org/oe-lkp/[email protected]
Fixes: f011c9cf04c0 ("io_uring/sqpoll: do not allow pinning outside of cpuset")
Signed-off-by: Jens Axboe <[email protected]>
---
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;
cpuset_cpus_allowed(current, &allowed_mask);
if (!cpumask_test_cpu(cpu, &allowed_mask))
goto err_sqpoll;
--
Jens Axboe
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid
2024-09-16 9:07 [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid Jens Axboe
@ 2024-09-16 9:11 ` MOESSBAUER, Felix
2024-09-16 9:12 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: MOESSBAUER, Felix @ 2024-09-16 9:11 UTC (permalink / raw)
To: [email protected], [email protected]
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 <[email protected]>
> Closes:
> https://lore.kernel.org/oe-lkp/[email protected]
> Fixes: f011c9cf04c0 ("io_uring/sqpoll: do not allow pinning outside
> of cpuset")
> Signed-off-by: Jens Axboe <[email protected]>
>
> ---
>
> 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 <[email protected]>
Felix
> cpuset_cpus_allowed(current, &allowed_mask);
> if (!cpumask_test_cpu(cpu, &allowed_mask))
> goto err_sqpoll;
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid
2024-09-16 9:11 ` MOESSBAUER, Felix
@ 2024-09-16 9:12 ` Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2024-09-16 9:12 UTC (permalink / raw)
To: MOESSBAUER, Felix, [email protected]
On 9/16/24 3:11 AM, MOESSBAUER, Felix wrote:
> 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 <[email protected]>
>> Closes:
>> https://lore.kernel.org/oe-lkp/[email protected]
>> Fixes: f011c9cf04c0 ("io_uring/sqpoll: do not allow pinning outside
>> of cpuset")
>> Signed-off-by: Jens Axboe <[email protected]>
>>
>> ---
>>
>> 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?
Probably not, but I felt saner just returning the old check so we don't
access the cpumask variable beyond the end.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-16 9:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-16 9:07 [PATCH] io_uring/sqpoll: retain test for whether the CPU is valid Jens Axboe
2024-09-16 9:11 ` MOESSBAUER, Felix
2024-09-16 9:12 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox