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

["Arnd Bergmann" <arnd@arndb.de>]

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