Re: [PATCH bpf-next v6 1/9] bpf: Allow each subprog having stack size of 512 bytes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/21/24 8:43 PM, Alexei Starovoitov wrote:
On Mon, Oct 21, 2024 at 8:21 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
          for (int i = 0; i < env->subprog_cnt; i++) {
-               if (!i || si[i].is_async_cb) {
-                       ret = check_max_stack_depth_subprog(env, i);
+               check_subprog = !i || (check_priv_stack ? si[i].is_cb : si[i].is_async_cb);
why?
This looks very suspicious.
This is to simplify jit. For example,
     main_prog   <=== main_prog_priv_stack_ptr
       subprog1  <=== there is a helper which has a callback_fn
                 <=== for example bpf_for_each_map_elem

         callback_fn
           subprog2

In callback_fn, we cannot simplify do
     r9 += stack_size_for_callback_fn
since r9 may have been clobbered between subprog1 and callback_fn.
That is why currently I allocate private_stack separately for callback_fn.

Alternatively we could do
     callback_fn_priv_stack_ptr = main_prog_priv_stack_ptr + off
where off equals to (stack size tree main_prog+subprog1).
I can do this approach too with a little more information in prog->aux.
WDYT?
I see. I think we're overcomplicating the verifier just to
be able to do 'r9 += stack' in the subprog.
The cases of async vs sync and directly vs kfunc/helper
(and soon with inlining of kfuncs) are getting too hard
to reason about.

I think we need to go back to the earlier approach
where every subprog had its own private stack and was
setting up r9 = my_priv_stack in the prologue.

Indeed, per private_stack per prog(subprog) will be much
simpler.


I suspect it's possible to construct a convoluted subprog
that calls itself a limited amount of time and the verifier allows that.
I feel it will be easier to detect just that condition
in the verifier and fallback to the normal stack.

Yes, I think check_max_stack_depth_subprog() should be able to detect subprog recursion.





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux