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] [thread-next>] [day] [month] [year] [list]

Message-Id: <eb4b4473-c75e-4bfa-9a16-19a5256a558d@app.fastmail.com>
Date: Wed, 02 Jul 2025 09:48:53 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Yonghong Song" <yonghong.song@...ux.dev>,
 "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 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.

(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