lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wi=M1MT8TT8oMsXcUTyNi+zgV56b6wNmhYZ5c=vaXiCOQ@mail.gmail.com>
Date: Fri, 9 Aug 2024 11:17:06 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Bert Karwatzki <spasswolf@....de>
Cc: mingo@...nel.org, akpm@...ux-foundation.org, 
	Oleg Nesterov <oleg@...hat.com>, Andy Lutomirski <luto@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Fenghua Yu <fenghua.yu@...el.com>, "H . Peter Anvin" <hpa@...or.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Uros Bizjak <ubizjak@...il.com>, linux-kernel@...r.kernel.org, peterz@...radead.org
Subject: Re: commit 81106b7e0b13 can break asm_int80_emulation on x86_64

On Fri, 9 Aug 2024 at 07:53, Bert Karwatzki <spasswolf@....de> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ