linux-kernel - Re: [PATCH] bpf: turn off sanitizer in do_misc

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] [day] [month] [year] [list]
Message-ID: <a8ea0565-e20d-4019-a64b-fa8020866411@linux.dev>
Date: Wed, 2 Jul 2025 07:14:05 -0700
From: Yonghong Song <yonghong.song@...ux.dev>
To: Arnd Bergmann <arnd@...db.de>, Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
 Arnd Bergmann <arnd@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Nathan Chancellor <nathan@...nel.org>,
 John Fastabend <john.fastabend@...il.com>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
 <eddyz87@...il.com>, Song Liu <song@...nel.org>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
 Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 Kumar Kartikeya Dwivedi <memxor@...il.com>,
 Luis Gerhorst <luis.gerhorst@....de>, bpf <bpf@...r.kernel.org>,
 LKML <linux-kernel@...r.kernel.org>, clang-built-linux <llvm@...ts.linux.dev>
Subject: Re: [PATCH] bpf: turn off sanitizer in do_misc_fixups for old clang



On 7/2/25 12:48 AM, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 23:28, Yonghong Song wrote:
>> On 7/1/25 1:45 PM, Andrii Nakryiko wrote:
>>> On Tue, Jul 1, 2025 at 1:03 PM Yonghong Song <yonghong.song@...ux.dev> wrote:
>>>> On 6/23/25 2:32 PM, Alexei Starovoitov wrote:
>>>>> On Fri, Jun 20, 2025 at 4:38 AM Arnd Bergmann <arnd@...nel.org> wrote:
>>>>>> From: Arnd Bergmann <arnd@...db.de>
>>>> I checked IR and found the following memory allocations which may contribute
>>>> excessive stack usage:
>>>>
>>>> attr.coerce1, i32 noundef %uattr_size) local_unnamed_addr #0 align 16 !dbg !19800 {
>>>> entry:
>>>>      %zext_patch.i = alloca [2 x %struct.bpf_insn], align 16, !DIAssignID !19854
>>>>      %rnd_hi32_patch.i = alloca [4 x %struct.bpf_insn], align 16, !DIAssignID !19855
>>>>      %cnt.i = alloca i32, align 4, !DIAssignID !19856
>>>>      %patch.i766 = alloca [3 x %struct.bpf_insn], align 16, !DIAssignID !19857
>>>>      %chk_and_sdiv.i = alloca [1 x %struct.bpf_insn], align 4, !DIAssignID !19858
>>>>      %chk_and_smod.i = alloca [1 x %struct.bpf_insn], align 4, !DIAssignID !19859
>>>>      %chk_and_div.i = alloca [4 x %struct.bpf_insn], align 16, !DIAssignID !19860
>>>>      %chk_and_mod.i = alloca [4 x %struct.bpf_insn], align 16, !DIAssignID !19861
>>>>      %chk_and_sdiv343.i = alloca [8 x %struct.bpf_insn], align 16, !DIAssignID !19862
>>>>      %chk_and_smod472.i = alloca [9 x %struct.bpf_insn], align 16, !DIAssignID !19863
>>>>      %desc.i = alloca %struct.bpf_jit_poke_descriptor, align 8, !DIAssignID !19864
>>>>      %target_size.i = alloca i32, align 4, !DIAssignID !19865
>>>>      %patch.i = alloca [2 x %struct.bpf_insn], align 16, !DIAssignID !19866
>>>>      %patch355.i = alloca [2 x %struct.bpf_insn], align 16, !DIAssignID !19867
>>>>      %ja.i = alloca %struct.bpf_insn, align 8, !DIAssignID !19868
>>>>      %ret_insn.i.i = alloca [8 x i32], align 16, !DIAssignID !19869
>>>>      %ret_prog.i.i = alloca [8 x i32], align 16, !DIAssignID !19870
>>>>      %fd.i = alloca i32, align 4, !DIAssignID !19871
>>>>      %log_true_size = alloca i32, align 4, !DIAssignID !19872
>>>> ...
>>>>
>>>> So yes, chk_and_{div,mod,sdiv,smod} consumes quite some stack and
>>>> can be coverted to runtime allocation but that is not enough for 1280
>>>> stack limit, we need to do more conversion from stack to memory
>>>> allocation. Will try to have uniform way to convert
>>>> 'alloca [<num> x %struct.bpf_insn]' to runtime allocation.
>>>>
>>> Do we need to go all the way to dynamic allocation? See env->insns_buf
>>> (which some parts of this function are already using for constructing
>>> instruction patch), let's just converge on that? It pre-allocates
>>> space for 32 instructions, should be sufficient for all the use cases,
>>> no?
>> Make sense. This is much better. Thanks!
> I'm not sure if that actually helps on the old clang version, as far
> as I understood it in my initial analysis, the problem in the
>
> struct bpf_insn chk_and_sdiv[] = {
>                                  /* [R,W]x sdiv 0 -> 0
>                                   * LLONG_MIN sdiv -1 -> LLONG_MIN
>                                   * INT_MIN sdiv -1 -> INT_MIN
>                                   */
>                                  BPF_MOV64_REG(BPF_REG_AX, insn->src_reg),
> ...
> }
>
> construct is not the chk_and_sdiv[] array itself but the
> struct initializer in the BPF_MOV64_REG() macro that leads to
> having two copies of the struct on the stack and then copying
> between them. In gcc or clang-18+, these all get folded
> into a single object on the stack.

See https://lore.kernel.org/bpf/20250702053332.1991516-1-yonghong.song@linux.dev/.
The above 'struct bpf_insn chk_and_sdiv[] = { ... }' will be removed so
there will not be stack consumption any more for it. Instead, we use
the scratch space in bpf_verifier_env.

>
> (Disclaimer: I don't understand anything about how clang
> actually works internally, the above is only speculation on
> my side, based on the assembler output)
>
>        Arnd