[<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