Re: commit 81106b7e0b13 can break asm_int80_emulation on x86_64

[Linus Torvalds]

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