Re: [PATCH bpf-next v1 6/7] bpf, s390: Use resolved pointer for timed may_goto

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3280 bytes --]

> commit 20356163d51e7b8551f96780e7e969a26767c5c4
> Author:     Siddharth Chintamaneni <sidchintamaneni@gmail.com>
> AuthorDate: Fri Sep 4 19:51:31 2026 +0000
> Commit:     Kernel Patches Daemon <kernel-patches-bot@fb.com>
> CommitDate: Fri Sep 4 13:03:02 2026 -0700
>
>     bpf, s390: Use resolved pointer for timed may_goto
>
>     The timed may_goto fixup now passes the resolved counter pointer through
>     BPF_REG_AX instead of a stack offset and BPF frame pointer pair.
>
>     Copy the pointer directly into the first argument register and update the
>     special calling convention documentation.
>
>     Fixes: b8efa810c1db ("s390/bpf: Add s390 JIT support for timed may_goto")
>     Reported-by: Jeremy Jean <jeremy.jean@oss.cyber.gouv.fr>
>     Link: https://lore.kernel.org/all/20260824213158.3755932-2-Jeremy.Jean@oss.cyber.gouv.fr/
>     Assisted-by: Copilot:gpt-5.6-sol
>     Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>

Should the Fixes: tag point at b8efa810c1db?

At that commit the s390 trampoline was correct for the ABI in force at the
time. kernel/bpf/fixups.c emitted `insn_buf[4] = BPF_MOV64_IMM(BPF_REG_AX,
stack_off_cnt)`, so %r12 held a signed byte offset and `lay
%r2,0(%r12,%r13)` correctly resolved it against the s390 BPF frame pointer
%r13. Nothing was wrong with b8efa810c1db.

What makes the old code wrong is the parent commit in this series,
d8319a04dafc ("bpf: Fix timed may_goto stack pointer for private stacks"),
which replaced that with:

  insn_buf[4] = BPF_MOV64_REG(BPF_REG_AX, BPF_REG_FP);
  insn_buf[5] = BPF_ALU64_IMM(BPF_ADD, BPF_REG_AX, stack_off_cnt)

so that %r12 now carries an already-resolved pointer. Only after that does
`lay %r2,0(%r12,%r13)` double-add the frame pointer.

If a stable maintainer or AUTOSEL picks 20356163d51e on the strength of
its Fixes: tag without also taking d8319a04dafc, s390 is left with `lgr
%r2,%r12` while the fixup still puts a raw offset (stack_off_cnt =
-stack_depth - 16, e.g. -528) in %r12:

  bpf_check_timed_may_goto(struct bpf_timed_may_goto *p)

then does `p->count` / `p->timestamp` loads and stores through a small
negative integer treated as a kernel pointer, which would trigger an oops
or memory corruption on every timed may_goto timeout.

The mirror case is equally bad: d8319a04dafc carries `Fixes:
e723608bf428` and will be selected for stable on its own, leaving s390
(and arm64/ppc64/riscv) double-adding the frame pointer.

The generic ABI change and its five arch consumers are not independently
correct, so either they belong in one commit, or the arch patches should
carry the same `Fixes: e723608bf428 ("bpf: Add verifier support for timed
may_goto")` and a `Cc: <stable@vger.kernel.org> # <sha>: <generic
commit>` prerequisite note. No Cc: stable and no prerequisite note is
present today.

Note: the same pattern is used by all four sibling commits (28bd77eb835b
x86, d35642770d98 arm64, 6e03227c5677 ppc64, 7053db7d3c0d riscv), so this
is a series-wide tagging decision rather than an s390-only slip.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33914557238