linux-kernel - Re: [PATCH] slab: Fix too strict alignment check in create

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: <CAMuHMdWHwXWtPWKnTii_JNqEn2hGQ3Un5fcJ9itH4t=VX-rONg@mail.gmail.com>
Date: Wed, 20 Nov 2024 16:50:06 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Guenter Roeck <linux@...ck-us.net>, Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>, 
	David Rientjes <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Roman Gushchin <roman.gushchin@...ux.dev>, 
	Hyeonggon Yoo <42.hyeyoo@...il.com>, Jens Axboe <axboe@...nel.dk>, 
	Pavel Begunkov <asml.silence@...il.com>, Mike Rapoport <rppt@...nel.org>, 
	Christian Brauner <brauner@...nel.org>, Kees Cook <keescook@...omium.org>, Jann Horn <jannh@...gle.com>, 
	linux-mm@...ck.org, io-uring@...r.kernel.org, linux-m68k@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] slab: Fix too strict alignment check in create_cache()

Hi Vlastimil,

On Wed, Nov 20, 2024 at 4:44 PM Vlastimil Babka <vbabka@...e.cz> wrote:
> On 11/20/24 16:14, Guenter Roeck wrote:
> > On 11/20/24 07:03, Vlastimil Babka wrote:
> >> On 11/20/24 13:49, Geert Uytterhoeven wrote:
> >>> On m68k, where the minimum alignment of unsigned long is 2 bytes:
> >>>
> >>>      Kernel panic - not syncing: __kmem_cache_create_args: Failed to create slab 'io_kiocb'. Error -22
> >>>      CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-atari-03776-g7eaa1f99261a #1783
> >>>      Stack from 0102fe5c:
> >>>         0102fe5c 00514a2b 00514a2b ffffff00 00000001 0051f5ed 00425e78 00514a2b
> >>>         0041eb74 ffffffea 00000310 0051f5ed ffffffea ffffffea 00601f60 00000044
> >>>         0102ff20 000e7a68 0051ab8e 004383b8 0051f5ed ffffffea 000000b8 00000007
> >>>         01020c00 00000000 000e77f0 0041e5f0 005f67c0 0051f5ed 000000b6 0102fef4
> >>>         00000310 0102fef4 00000000 00000016 005f676c 0060a34c 00000010 00000004
> >>>         00000038 0000009a 01000000 000000b8 005f668e 0102e000 00001372 0102ff88
> >>>      Call Trace: [<00425e78>] dump_stack+0xc/0x10
> >>>       [<0041eb74>] panic+0xd8/0x26c
> >>>       [<000e7a68>] __kmem_cache_create_args+0x278/0x2e8
> >>>       [<000e77f0>] __kmem_cache_create_args+0x0/0x2e8
> >>>       [<0041e5f0>] memset+0x0/0x8c
> >>>       [<005f67c0>] io_uring_init+0x54/0xd2
> >>>
> >>> The minimal alignment of an integral type may differ from its size,
> >>> hence is not safe to assume that an arbitrary freeptr_t (which is
> >>> basically an unsigned long) is always aligned to 4 or 8 bytes.
> >>>
> >>> As nothing seems to require the additional alignment, it is safe to fix
> >>> this by relaxing the check to the actual minimum alignment of freeptr_t.
> >>>
> >>> Fixes: aaa736b186239b7d ("io_uring: specify freeptr usage for SLAB_TYPESAFE_BY_RCU io_kiocb cache")
> >>> Fixes: d345bd2e9834e2da ("mm: add kmem_cache_create_rcu()")
> >>> Reported-by: Guenter Roeck <linux@...ck-us.net>
> >>> Closes: https://lore.kernel.org/37c588d4-2c32-4aad-a19e-642961f200d7@roeck-us.net
> >>> Signed-off-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> >>
> >> Thanks, will add it to slab pull for 6.13.
> >>
> >>> ---
> >>>   mm/slab_common.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/mm/slab_common.c b/mm/slab_common.c
> >>> index 893d320599151845..f2f201d865c108bd 100644
> >>> --- a/mm/slab_common.c
> >>> +++ b/mm/slab_common.c
> >>> @@ -230,7 +230,7 @@ static struct kmem_cache *create_cache(const char *name,
> >>>     if (args->use_freeptr_offset &&
> >>>         (args->freeptr_offset >= object_size ||
> >>>          !(flags & SLAB_TYPESAFE_BY_RCU) ||
> >>> -        !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
> >>> +        !IS_ALIGNED(args->freeptr_offset, __alignof(freeptr_t))))
> >>
> >> Seems only bunch of places uses __alignof but many use __alignoff__ and this
> >> also is what seems to be documented?
> >
> > __alignoff__ -> __alignof__
>
> Yeah I meant __alignof__
> Will chage it locally then.

Thank you!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds