Re: [PATCH bpf-next v4 10/18] bpf: Fix interaction between stack argument PTR_TO_STACK and dead slot poisoning

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

On 4/15/26 3:32 PM, Amery Hung wrote:
> On Sat, Apr 11, 2026 at 10:01 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> The "poison dead stack slots" mechanism (commit 2cb27158adb3) uses
>> static liveness analysis to identify dead stack slots and poisons them
>> as a safety check. However, the static liveness pass cannot track
>> indirect stack references through pointers passed via stack arguments.
>>
>> For register-passed PTR_TO_STACK (e.g., R1 = fp-8 passed to a static
>> subprog), the liveness abstract tracker carries frame/offset info
>> through registers. When the callee dereferences R1, the tracker
>> attributes the read to the parent frame's stack slot, correctly marking
>> it alive. So no poisoning issue arises.
>>
>> For stack-argument-passed PTR_TO_STACK (e.g., fp-8 stored via
>> *(r12-8) = r1), the value goes through BPF_REG_STACK_ARG_BASE (r12)
>> which the liveness pass does not track. When the callee loads the
>> pointer from its incoming stack arg and dereferences it, the liveness
>> pass cannot attribute the read back to the parent frame. The parent's
>> stack slot is determined dead and poisoned before the callee even
>> starts. The callee's subsequent dereference then fails with "slot
>> poisoned by dead code elimination".
>>
>> Fix this by allowing STACK_POISON reads in check_stack_read_fixed_off()
>> when the read targets a parent frame's stack (reg_state != state).
>> Same-frame STACK_POISON reads remain rejected to preserve the safety
>> check for real liveness bugs. Cross-frame reads are safe to allow
>> because:
>>    - The pointer to the parent's stack was already validated by the
>>      verifier.
>>    - The slot contained valid data before being (incorrectly) poisoned.
>>    - The read returns an unknown scalar, which is conservative.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
> While liveness of stack arg handled differently, can R12 base
> arguments cause some OOB in liveness.c? For example, can
> arg_track_xfer() reference at_out[12] while at_out is defined in
> compute_subprog_args() as struct arg_track at_out[MAX_BPF_REG]?

Yes. The v4 does not have this issue as it does not have arg_track_xfer()
yet when I posted it. But on top of the latest master, this is indeed an
issue and I am aware of this. Thanks for pointing it out!

>
>>   kernel/bpf/verifier.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index e664d924e8d4..bfeecd73e66e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5764,6 +5764,13 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>>                                          }
>>                                          if (type == STACK_INVALID && env->allow_uninit_stack)
>>                                                  continue;
>> +                                       /*
>> +                                        * Cross-frame reads may hit slots poisoned by dead code elimination.
>> +                                        * Static liveness can't track indirect references through pointers,
>> +                                        * so allow the read conservatively.
>> +                                        */
>> +                                       if (type == STACK_POISON && reg_state != state)
>> +                                               continue;
>>                                          if (type == STACK_POISON) {
>>                                                  verbose(env, "reading from stack off %d+%d size %d, slot poisoned by dead code elimination\n",
>>                                                          off, i, size);
>> @@ -5819,6 +5826,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>>                                  continue;
>>                          if (type == STACK_INVALID && env->allow_uninit_stack)
>>                                  continue;
>> +                       if (type == STACK_POISON && reg_state != state)
>> +                               continue;
>>                          if (type == STACK_POISON) {
>>                                  verbose(env, "reading from stack off %d+%d size %d, slot poisoned by dead code elimination\n",
>>                                          off, i, size);
>> --
>> 2.52.0
>>
>>