Re: [PATCH] bpf: turn off sanitizer in do_misc_fixups for old clang

Yonghong Song <yonghong.song@xxxxxxxxx> · Wed, 2 Jul 2025 07:14:05 -0700

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@xxxxxxxxx> wrote:
On 6/23/25 2:32 PM, Alexei Starovoitov wrote:
On Fri, Jun 20, 2025 at 4:38 AM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
From: Arnd Bergmann <arnd@xxxxxxxx>
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@xxxxxxxxx/.
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