Re: commit 81106b7e0b13 can break asm_int80_emulation on x86_64

From: Linus Torvalds
Date: Fri Aug 09 2024 - 14:17:37 EST


On Fri, 9 Aug 2024 at 07:53, Bert Karwatzki <spasswolf@xxxxxx> wrote:
>
> So the problem seems to be that the kmem_cache object *s has usersize 0. This
> should be impossible in theory as kmem_cache_create_usercopy() should print
> a warning in case of (!usersize && useroffset).

Following along from your original report:

usercopy: Kernel memory overwrite attempt detected to SLUB object
'task_struct' (offset 3072, size 160)!

IOW, the user copy code is unhappy about copying data from user mode
into the task_struct allocation.

Anyway, on x86-64 with that commit we now just do

arch_thread_struct_whitelist(unsigned long *offset, unsigned long *size)
{
*offset = 0;
*size = 0;
}

and that seems to be the bug.

It's still allocated together with that 'struct task_struct', even if
it's no longer inside the 'struct thread_struct'.

Ingo? I think it needs to be something like

*offset = sizeof(struct task_struct)
- offsetof(struct task_struct, thread)
+ offsetof(struct fpu, __fpstate.regs);
*size = fpu_kernel_cfg.default_size;

and the commit message of

The fpu_thread_struct_whitelist() quirk to hardened usercopy can be removed,
now that the FPU structure is not embedded in the task struct anymore, which
reduces text footprint a bit.

is clearly not true. No, it's not embedded in 'struct task_struct',
but it *is* still allocated together with it in the same slab if I
read the code correctly (ie this part

static void __init fpu__init_task_struct_size(void)
{
int task_size = sizeof(struct task_struct);

task_size += sizeof(struct fpu);
..
arch_task_struct_size = task_size;


is because it's still all one single slab allocation.

Linus