Re: yet another approach Was: [PATCH bpf-next v3 4/5] bpf, x86: Add jit support for private stack

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

On 10/1/24 7:16 PM, Kumar Kartikeya Dwivedi wrote:
> On Wed, 2 Oct 2024 at 03:26, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Tue, Oct 1, 2024 at 5:23 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>>> Makes sense, though will we have cases where hierarchical scheduling
>>> attaches the same prog at different points of the hierarchy?
>> I'm not sure anyone was asking for such a use case.
> I wondered because why would you then need a limit of 4 (say instead
> of disallowing it)?
>
>>> Then the
>>> limit of 4 may not be enough (e.g. say with cgroup nested levels > 4).
>> Well, 4 was the number from TJ.
>>
> Ok, then let's assume 4 would be enough.
>
>> Anyway the proposed pseudo code:
>>
>> __bpf_prog_enter_recur_limited()
>> {
>>    cnt = this_cpu_inc_return(*(prog->active));
>>    if (cnt > 4) {
>>       inc_miss
>>       return 0;
>>    }
>>   // pass cnt into bpf prog somehow, like %rdx ?
>>   // or re-read prog->active from prog
>> }
>>
>>
>> then in the prologue emit:
>>
>> push rbp
>> mov rbp, rsp
>> if %rdx == 1
>>     // main prog is called for the first time
>>     mov rsp, pcpu_priv_stack_top

This sounds good in high level. I still need to figure out
'if %rdx == 1' part and how to implement this.

>> else
>>     // 2+nd time main prog is called or 1+ time subprog
>>    sub rsp, stack_size
>>    if rsp < pcpu_priv_stack_bottom
>>      goto exit  // stack is too small, exit
>> fi
> I think we need just the second part for subprogs, right?
> Since rdx is R3 (arg into subprog).
> I guess that's what you meant in the pseudocode.
> But otherwise sounds good.
> The benefit with stack probing is we don't exactly limit to 4 cases.
>
> Another option instead of the branch in main prog is to divide in 4
> slots (as you said before) and choose the slot based on cnt.
> But then we're stuck with a max limit of 4. Since we're allocating
> stack size of bpf + extra (which I guess is 8K?). rdx can be used to
> pass in the priv_stack address of the right slot.
>
> So I think the probing version seems better. We can probably pass in
> rdx = priv_stack and then test and cmov instead for main prog.

Yes, we do not need to limit to 4, checking rsp < pcpu_priv_stack_bottom
should be okay.

>
>> Since stack bottom/top are known at JIT time we can
>> generate reliable stack overflow checks.
>> Much better than guard pages and -fstack-protector.
>> The prog can alloc percpu
>> (stack size of main prog + subprogs + extra) * 4
> extra will be 8K, I guess (same as kernel stack size)?
> Just confirming.
>
>> and it likely will be enough.
>> If not, the stack protection will gently exit the prog
>> when the stack is too deep.
> I like this stack probing version, since there's no hard limit on the
> number of recursions, and it's safe against stack overflow as well.
>
>> kfunc won't have such a check, so we need a buffer zone.
>> Can have a guard page too, but feels like overkill.
> I was leaning toward saying yes for a guard page, since we'll atleast
> have a hard error instead of random corruption if the kfunc goes
> beyond the bottom after probing succeeds.
>
> But the better way might be doing if rsp < pcpu_priv_stack_bottom +
> 8K, so we leave max headroom we reserve for kernel stuff (or say add
> 4K instead, which should be good enough), and then skip execution.