Re: [PATCH v2] bpf: make function do_misc_fixups as noinline_for_stack

[Yonghong Song <yonghong.song@linux.dev>]

On 7/24/24 11:18 PM, Hao Peng wrote:
> On Sat, Jul 13, 2024 at 12:43 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Wed, Jul 10, 2024 at 10:45 PM <flyingpenghao@gmail.com> wrote:
>>>
>>> By tracing the call chain, we found that do_misc_fixups consumed a lot
>>> of stack space. mark it as noinline_for_stack to prevent it from spreading
>>> to bpf_check's stack size.
>> ...
>>> -static int do_misc_fixups(struct bpf_verifier_env *env)
>>> +static noinline_for_stack int do_misc_fixups(struct bpf_verifier_env *env)
>> Now we're getting somewhere, but this is not a fix.
>> It may shut up the warn, but it will only increase the total stack usage.
>> Looking at C code do_misc_fixups() needs ~200 bytes worth of stack
>> space for insn_buf[16] and spill/fill.
>> That's far from the artificial 2k limit.
>>
>> Please figure out what exact variable is causing kasan to consume
>> so much stack. You may need to analyze compiler internals and
>> do more homework.
>> What is before/after stack usage ? with and without kasan?
>> With gcc try
>> +CFLAGS_verifier.o += -fstack-usage
>>
>> I see:
>> sort -k2 -n kernel/bpf/verifier.su |tail -10
>> ../kernel/bpf/verifier.c:13087:12:adjust_ptr_min_max_vals    240
>> dynamic,bounded
>> ../kernel/bpf/verifier.c:20804:12:do_check_common    248    dynamic,bounded
>> ../kernel/bpf/verifier.c:19151:12:convert_ctx_accesses    256    static
>> ../kernel/bpf/verifier.c:7450:12:check_mem_reg    256    static
>> ../kernel/bpf/verifier.c:7482:12:check_kfunc_mem_size_reg    256    static
>> ../kernel/bpf/verifier.c:10268:12:check_helper_call.isra    272
>> dynamic,bounded
>> ../kernel/bpf/verifier.c:21562:5:bpf_check    296    dynamic,bounded
>> ../kernel/bpf/verifier.c:19860:12:do_misc_fixups    320    static
>> ../kernel/bpf/verifier.c:13991:12:adjust_reg_min_max_vals    392    static
>> ../kernel/bpf/verifier.c:12280:12:check_kfunc_call.isra    408
>> dynamic,bounded
>>
>> do_misc_fixups() is not the smallest, but not that large either.
>>
> If I use gcc, I get the same result as you, but if I use llvm to build
> the kernel, the result is like this：
> # sort -k2 -n kernel/bpf/verifier.su | tail -10
> kernel/bpf/verifier.c:14026:adjust_reg_min_max_vals     440     static
> kernel/bpf/verifier.c:7432:check_mem_reg        440     static
> kernel/bpf/verifier.c:15955:check_cfg   472     static
> kernel/bpf/verifier.c:7464:check_kfunc_mem_size_reg     472     static
> kernel/bpf/verifier.c:15104:check_cond_jmp_op   504     static
> kernel/bpf/verifier.c:4166:__mark_chain_precision       504     static
> kernel/bpf/verifier.c:10239:check_helper_call   536     static
> kernel/bpf/verifier.c:17744:do_check    792     static
> kernel/bpf/verifier.c:12248:check_kfunc_call    984     static
> kernel/bpf/verifier.c:21486:bpf_check   2456    static
>
> Obviously, do_misc_fixups is automatically inlined into bpf_check.
> So adding noinline_for_stack to the do_misc_fixups function is a solution.

Looks like you are building your own kernel with KASAN.
You can change config CONFIG_FRAME_WARN value. In your config file you
have CONFIG_FRAME_WARN=2048. You can change it to
CONFIG_FRAME_WARN=4096 which should fix the issue.

>
> Thanks.
>
>> Do in-depth analysis instead of silencing the warn.
>>
>> pw-bot: cr