Stable backport request: skbuff: Proactively round up to kmalloc bucket size

Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Daniel Díaz]

Hello!

Would the stable maintainers please consider backporting the following
commit to the 6.1? We are trying to build gki_defconfig (plus a few
extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
Bisection has pointed here.

We have verified that cherry-picking this patch on top of v6.1.29
applies cleanly and allows the kernel to boot.

commit 12d6c1d3a2ad0c199ec57c201cdc71e8e157a232
Author: Kees Cook <keescook@chromium.org>
Date:   Tue Oct 25 15:39:35 2022 -0700

    skbuff: Proactively round up to kmalloc bucket size

    Instead of discovering the kmalloc bucket size _after_ allocation, round
    up proactively so the allocation is explicitly made for the full size,
    allowing the compiler to correctly reason about the resulting size of
    the buffer through the existing __alloc_size() hint.

    This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the
    coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain
    back the __alloc_size() hints that were temporarily reverted in commit
    93dd04ab0b2b ("slab: remove __alloc_size attribute from
__kmalloc_track_caller")

    Cc: "David S. Miller" <davem@davemloft.net>
    Cc: Eric Dumazet <edumazet@google.com>
    Cc: Jakub Kicinski <kuba@kernel.org>
    Cc: Paolo Abeni <pabeni@redhat.com>
    Cc: netdev@vger.kernel.org
    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Cc: Nick Desaulniers <ndesaulniers@google.com>
    Cc: David Rientjes <rientjes@google.com>
    Acked-by: Vlastimil Babka <vbabka@suse.cz>
    Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221021234713.you.031-kees@kernel.org/
    Signed-off-by: Kees Cook <keescook@chromium.org>
    Link: https://lore.kernel.org/r/20221025223811.up.360-kees@kernel.org
    Signed-off-by: Paolo Abeni <pabeni@redhat.com>


Thanks and greetings!

Daniel Díaz
daniel.diaz@linaro.org

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Greg Kroah-Hartman]

On Mon, May 22, 2023 at 12:23:50PM -0600, Daniel Díaz wrote:
> Hello!
> 
> Would the stable maintainers please consider backporting the following
> commit to the 6.1? We are trying to build gki_defconfig (plus a few
> extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
> Bisection has pointed here.

I do not see a "gki_defconfig" in the kernel tree, is this just
out-of-tree stuff?

If so, why not just add this to your out-of-tree stuff?

> We have verified that cherry-picking this patch on top of v6.1.29
> applies cleanly and allows the kernel to boot.

So what is breaking that requires this to fix the problem?  What is the
problem?

> 
> commit 12d6c1d3a2ad0c199ec57c201cdc71e8e157a232
> Author: Kees Cook <keescook@chromium.org>
> Date:   Tue Oct 25 15:39:35 2022 -0700
> 
>     skbuff: Proactively round up to kmalloc bucket size
> 
>     Instead of discovering the kmalloc bucket size _after_ allocation, round
>     up proactively so the allocation is explicitly made for the full size,
>     allowing the compiler to correctly reason about the resulting size of
>     the buffer through the existing __alloc_size() hint.
> 
>     This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the
>     coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain
>     back the __alloc_size() hints that were temporarily reverted in commit
>     93dd04ab0b2b ("slab: remove __alloc_size attribute from
> __kmalloc_track_caller")
> 
>     Cc: "David S. Miller" <davem@davemloft.net>
>     Cc: Eric Dumazet <edumazet@google.com>
>     Cc: Jakub Kicinski <kuba@kernel.org>
>     Cc: Paolo Abeni <pabeni@redhat.com>
>     Cc: netdev@vger.kernel.org
>     Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Cc: Nick Desaulniers <ndesaulniers@google.com>
>     Cc: David Rientjes <rientjes@google.com>
>     Acked-by: Vlastimil Babka <vbabka@suse.cz>
>     Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221021234713.you.031-kees@kernel.org/
>     Signed-off-by: Kees Cook <keescook@chromium.org>
>     Link: https://lore.kernel.org/r/20221025223811.up.360-kees@kernel.org
>     Signed-off-by: Paolo Abeni <pabeni@redhat.com>

This feels like a new feature, why would a 6.1.y system need it?  What
commit id does it fix?

thanks,

greg k-h

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Nick Desaulniers]

On Mon, May 22, 2023 at 11:24 AM Daniel Díaz <daniel.diaz@linaro.org> wrote:
>
> Hello!
>
> Would the stable maintainers please consider backporting the following
> commit to the 6.1? We are trying to build gki_defconfig (plus a few

Does android's gki_defconfig fail to boot on the `android14-6.1`
branch of https://android.googlesource.com/kernel/common?

(i.e. downstream branch from linux stable's linux-6.1.y)?

We just ran CI successfully on that branch 10 hours ago.
https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/5042504560/jobs/9045030265

Do you have more information on the observed boot failure? (panic splat?)

> extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
> Bisection has pointed here.
>
> We have verified that cherry-picking this patch on top of v6.1.29
> applies cleanly and allows the kernel to boot.
>
> commit 12d6c1d3a2ad0c199ec57c201cdc71e8e157a232
> Author: Kees Cook <keescook@chromium.org>
> Date:   Tue Oct 25 15:39:35 2022 -0700
>
>     skbuff: Proactively round up to kmalloc bucket size
>
>     Instead of discovering the kmalloc bucket size _after_ allocation, round
>     up proactively so the allocation is explicitly made for the full size,
>     allowing the compiler to correctly reason about the resulting size of
>     the buffer through the existing __alloc_size() hint.
>
>     This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the
>     coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain
>     back the __alloc_size() hints that were temporarily reverted in commit
>     93dd04ab0b2b ("slab: remove __alloc_size attribute from
> __kmalloc_track_caller")
>
>     Cc: "David S. Miller" <davem@davemloft.net>
>     Cc: Eric Dumazet <edumazet@google.com>
>     Cc: Jakub Kicinski <kuba@kernel.org>
>     Cc: Paolo Abeni <pabeni@redhat.com>
>     Cc: netdev@vger.kernel.org
>     Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Cc: Nick Desaulniers <ndesaulniers@google.com>
>     Cc: David Rientjes <rientjes@google.com>
>     Acked-by: Vlastimil Babka <vbabka@suse.cz>
>     Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221021234713.you.031-kees@kernel.org/
>     Signed-off-by: Kees Cook <keescook@chromium.org>
>     Link: https://lore.kernel.org/r/20221025223811.up.360-kees@kernel.org
>     Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
>
> Thanks and greetings!
>
> Daniel Díaz
> daniel.diaz@linaro.org



-- 
Thanks,
~Nick Desaulniers

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Daniel Díaz]

Hello!

On Mon, 22 May 2023 at 12:37, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Mon, May 22, 2023 at 11:24 AM Daniel Díaz <daniel.diaz@linaro.org> wrote:
> >
> > Hello!
> >
> > Would the stable maintainers please consider backporting the following
> > commit to the 6.1? We are trying to build gki_defconfig (plus a few
>
> Does android's gki_defconfig fail to boot on the `android14-6.1`
> branch of https://android.googlesource.com/kernel/common?
>
> (i.e. downstream branch from linux stable's linux-6.1.y)?
>
> We just ran CI successfully on that branch 10 hours ago.
> https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/5042504560/jobs/9045030265
>
> Do you have more information on the observed boot failure? (panic splat?)

Apologies if it sounded like we were trying to boot an Android kernel.
Let me clarify: We're booting v6.1.29 from linux-stable/linux-6.1.y.

This is what we get under Qemu-arm64 for v6.1.29 with Clang 16:
-----8<-----
  Unexpected kernel BRK exception at EL1
  Internal error: BRK handler: 00000000f2000001 [#1] PREEMPT SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.29 #1
  Hardware name: linux,dummy-virt (DT)
  pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : pskb_expand_head+0x448/0x480
  lr : pskb_expand_head+0x13c/0x480
  sp : ffff80000802b850
  x29: ffff80000802b860 x28: 00000000000002c0 x27: 0000000000000ec0
  x26: ffff0000c02c8ec0 x25: ffff0000c02c8000 x24: 00000000000128c0
  x23: ffff0000c030e800 x22: ffff0000c030e800 x21: 0000000000000240
  x20: 0000000000000000 x19: ffff0000c085e900 x18: ffff800008021068
  x17: 00000000ad6b63b6 x16: 00000000ad6b63b6 x15: 0001001c00070038
  x14: 0000000c00020008 x13: 00882cc00000ffff x12: 0000000000000000
  x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000001
  x8 : ffff0000c030eac0 x7 : 0000000000000000 x6 : 0000000000000000
  x5 : ffff0000c030eaf0 x4 : ffff0000ff7abd10 x3 : 0000000000001740
  x2 : ffff0000c02c8000 x1 : 0000000000000000 x0 : 0000000000000000
  Call trace:
   pskb_expand_head+0x448/0x480
   netlink_trim+0xa0/0xc8
   netlink_broadcast+0x54/0x764
   genl_ctrl_event+0x21c/0x37c
   genl_register_family+0x628/0x708
   thermal_netlink_init+0x28/0x3c
   thermal_init+0x28/0xec
   do_one_initcall+0xfc/0x358
   do_initcall_level+0xd8/0x1b4
   do_initcalls+0x64/0xa8
   do_basic_setup+0x2c/0x3c
   kernel_init_freeable+0x118/0x198
   kernel_init+0x30/0x1c0
   ret_from_fork+0x10/0x20
  Code: f9406679 38776b28 3707eba8 17ffff67 (d4200020)
  ---[ end trace 0000000000000000 ]---
  Kernel panic - not syncing: BRK handler: Fatal exception
  SMP: stopping secondary CPUs
----->8-----

Here's a link to that test, with all artifacts:
https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/daniel/tests/2QA2CVTTvG6KZETMUyZCNgS8koR

This can be reproduced locally via Tuxrun:
-----8<-----
#pip3 install -U tuxrun
tuxrun --runtime podman \
  --device qemu-arm64 \
  --image docker.io/lavasoftware/lava-dispatcher:2023.01.0020.gc1598238f \
  --boot-args rw \
  --kernel https://storage.tuxsuite.com/public/linaro/daniel/builds/2QA2CHQUpqKe27FyMZrBNILVwXi/Image.gz
\
  --modules https://storage.tuxsuite.com/public/linaro/daniel/builds/2QA2CHQUpqKe27FyMZrBNILVwXi/modules.tar.xz
\
  --rootfs https://storage.tuxboot.com/debian/bookworm/arm64/rootfs.ext4.xz
----->8-----

This is vanilla v6.1.29 with no extra patches, just this kernel configuration:
  https://storage.tuxsuite.com/public/linaro/daniel/builds/2QA2CHQUpqKe27FyMZrBNILVwXi/config

Greetings!

Daniel Díaz
daniel.diaz@linaro.org



> > extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
> > Bisection has pointed here.
> >
> > We have verified that cherry-picking this patch on top of v6.1.29
> > applies cleanly and allows the kernel to boot.
> >
> > commit 12d6c1d3a2ad0c199ec57c201cdc71e8e157a232
> > Author: Kees Cook <keescook@chromium.org>
> > Date:   Tue Oct 25 15:39:35 2022 -0700
> >
> >     skbuff: Proactively round up to kmalloc bucket size
> >
> >     Instead of discovering the kmalloc bucket size _after_ allocation, round
> >     up proactively so the allocation is explicitly made for the full size,
> >     allowing the compiler to correctly reason about the resulting size of
> >     the buffer through the existing __alloc_size() hint.
> >
> >     This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the
> >     coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain
> >     back the __alloc_size() hints that were temporarily reverted in commit
> >     93dd04ab0b2b ("slab: remove __alloc_size attribute from
> > __kmalloc_track_caller")
> >
> >     Cc: "David S. Miller" <davem@davemloft.net>
> >     Cc: Eric Dumazet <edumazet@google.com>
> >     Cc: Jakub Kicinski <kuba@kernel.org>
> >     Cc: Paolo Abeni <pabeni@redhat.com>
> >     Cc: netdev@vger.kernel.org
> >     Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >     Cc: Nick Desaulniers <ndesaulniers@google.com>
> >     Cc: David Rientjes <rientjes@google.com>
> >     Acked-by: Vlastimil Babka <vbabka@suse.cz>
> >     Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221021234713.you.031-kees@kernel.org/
> >     Signed-off-by: Kees Cook <keescook@chromium.org>
> >     Link: https://lore.kernel.org/r/20221025223811.up.360-kees@kernel.org
> >     Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >
> >
> > Thanks and greetings!
> >
> > Daniel Díaz
> > daniel.diaz@linaro.org
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Vlastimil Babka]

On 5/22/23 20:23, Daniel Díaz wrote:
> Hello!
> 
> Would the stable maintainers please consider backporting the following
> commit to the 6.1? We are trying to build gki_defconfig (plus a few
> extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
> Bisection has pointed here.

You mean the bisection was done to find the first "good" commit between 6.1
and e.g. 6.3?

As others said, this commit wasn't expected to be a fix to a known bug.
Maybe you found one that we didn't know of, or it might be accidentaly
masking some other bug.

> We have verified that cherry-picking this patch on top of v6.1.29
> applies cleanly and allows the kernel to boot.
> 
> commit 12d6c1d3a2ad0c199ec57c201cdc71e8e157a232
> Author: Kees Cook <keescook@chromium.org>
> Date:   Tue Oct 25 15:39:35 2022 -0700
> 
>     skbuff: Proactively round up to kmalloc bucket size
> 
>     Instead of discovering the kmalloc bucket size _after_ allocation, round
>     up proactively so the allocation is explicitly made for the full size,
>     allowing the compiler to correctly reason about the resulting size of
>     the buffer through the existing __alloc_size() hint.
> 
>     This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the
>     coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain
>     back the __alloc_size() hints that were temporarily reverted in commit
>     93dd04ab0b2b ("slab: remove __alloc_size attribute from
> __kmalloc_track_caller")
> 
>     Cc: "David S. Miller" <davem@davemloft.net>
>     Cc: Eric Dumazet <edumazet@google.com>
>     Cc: Jakub Kicinski <kuba@kernel.org>
>     Cc: Paolo Abeni <pabeni@redhat.com>
>     Cc: netdev@vger.kernel.org
>     Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Cc: Nick Desaulniers <ndesaulniers@google.com>
>     Cc: David Rientjes <rientjes@google.com>
>     Acked-by: Vlastimil Babka <vbabka@suse.cz>
>     Link: https://patchwork.kernel.org/project/netdevbpf/patch/20221021234713.you.031-kees@kernel.org/
>     Signed-off-by: Kees Cook <keescook@chromium.org>
>     Link: https://lore.kernel.org/r/20221025223811.up.360-kees@kernel.org
>     Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> 
> Thanks and greetings!
> 
> Daniel Díaz
> daniel.diaz@linaro.org

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Daniel Díaz]

Hello!

On Tue, 23 May 2023 at 00:28, Vlastimil Babka <vbabka@suse.cz> wrote:
> On 5/22/23 20:23, Daniel Díaz wrote:
> > Hello!
> >
> > Would the stable maintainers please consider backporting the following
> > commit to the 6.1? We are trying to build gki_defconfig (plus a few
> > extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
> > Bisection has pointed here.
>
> You mean the bisection was done to find the first "good" commit between 6.1
> and e.g. 6.3?
>
> As others said, this commit wasn't expected to be a fix to a known bug.
> Maybe you found one that we didn't know of, or it might be accidentaly
> masking some other bug.

How interesting! Yes, we happened to run a bisection between v6.1 and
v6.3 and we found where it started working with the following
configuration:
  https://storage.tuxsuite.com/public/linaro/daniel/builds/2QA2CHQUpqKe27FyMZrBNILVwXi/config

With that patch on top of v6.1.29 it boots fine under Qemu-arm64; as
v6.1.y stands, it panics with this:
-----8<-----
  Unexpected kernel BRK exception at EL1
  Internal error: BRK handler: 00000000f2000001 [#1] PREEMPT SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.29 #1
  Hardware name: linux,dummy-virt (DT)
  pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : pskb_expand_head+0x448/0x480
  lr : pskb_expand_head+0x13c/0x480
  sp : ffff80000802b850
  x29: ffff80000802b860 x28: 00000000000002c0 x27: 0000000000000ec0
  x26: ffff0000c02c8ec0 x25: ffff0000c02c8000 x24: 00000000000128c0
  x23: ffff0000c030e800 x22: ffff0000c030e800 x21: 0000000000000240
  x20: 0000000000000000 x19: ffff0000c085e900 x18: ffff800008021068
  x17: 00000000ad6b63b6 x16: 00000000ad6b63b6 x15: 0001001c00070038
  x14: 0000000c00020008 x13: 00882cc00000ffff x12: 0000000000000000
  x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000001
  x8 : ffff0000c030eac0 x7 : 0000000000000000 x6 : 0000000000000000
  x5 : ffff0000c030eaf0 x4 : ffff0000ff7abd10 x3 : 0000000000001740
  x2 : ffff0000c02c8000 x1 : 0000000000000000 x0 : 0000000000000000
  Call trace:
   pskb_expand_head+0x448/0x480
   netlink_trim+0xa0/0xc8
   netlink_broadcast+0x54/0x764
   genl_ctrl_event+0x21c/0x37c
   genl_register_family+0x628/0x708
   thermal_netlink_init+0x28/0x3c
   thermal_init+0x28/0xec
   do_one_initcall+0xfc/0x358
   do_initcall_level+0xd8/0x1b4
   do_initcalls+0x64/0xa8
   do_basic_setup+0x2c/0x3c
   kernel_init_freeable+0x118/0x198
   kernel_init+0x30/0x1c0
   ret_from_fork+0x10/0x20
  Code: f9406679 38776b28 3707eba8 17ffff67 (d4200020)
  ---[ end trace 0000000000000000 ]---
  Kernel panic - not syncing: BRK handler: Fatal exception
  SMP: stopping secondary CPUs
----->8-----

Greetings!

Daniel Díaz
daniel.diaz@linaro.org

Re: Stable backport request: skbuff: Proactively round up to kmalloc bucket size

[Kees Cook]

On May 23, 2023 8:52:53 PM PDT, "Daniel Díaz" <daniel.diaz@linaro.org> wrote:
>Hello!
>
>On Tue, 23 May 2023 at 00:28, Vlastimil Babka <vbabka@suse.cz> wrote:
>> On 5/22/23 20:23, Daniel Díaz wrote:
>> > Hello!
>> >
>> > Would the stable maintainers please consider backporting the following
>> > commit to the 6.1? We are trying to build gki_defconfig (plus a few
>> > extras) on Arm64 and test it under Qemu-arm64, but it fails to boot.
>> > Bisection has pointed here.
>>
>> You mean the bisection was done to find the first "good" commit between 6.1
>> and e.g. 6.3?
>>
>> As others said, this commit wasn't expected to be a fix to a known bug.
>> Maybe you found one that we didn't know of, or it might be accidentaly
>> masking some other bug.
>
>How interesting! Yes, we happened to run a bisection between v6.1 and
>v6.3 and we found where it started working with the following
>configuration:
>  https://storage.tuxsuite.com/public/linaro/daniel/builds/2QA2CHQUpqKe27FyMZrBNILVwXi/config

Ah yes, from CONFIG_UBSAN_BOUNDS=y and CONFIG_UBSAN_TRAP=y

This was a known issue in upstream, oddly only exposed on arm64. Something re-broke with __alloc_size after commit 93dd04ab0b2b had tried to work around it. I didn't think any kernel released with this broken, though, so perhaps what broke it got added to -stable?

>With that patch on top of v6.1.29 it boots fine under Qemu-arm64; as
>v6.1.y stands, it panics with this:

It should be fine to backport the patch, IMO.


-- 
Kees Cook