Thread (12 messages) flat view 12 messages, 2 authors, 2d ago

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

From: bot+bpf-ci@kernel.org
Date: 2026-09-04 20:33:50
Also in: bpf, linux-riscv, linux-s390

commit 20356163d51e7b8551f96780e7e969a26767c5c4
Author:     Siddharth Chintamaneni [off-list ref]
AuthorDate: Fri Sep 4 19:51:31 2026 +0000
Commit:     Kernel Patches Daemon [off-list ref]
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 [off-list ref]
    Link: https://lore.kernel.org/all/20260824213158.3755932-2-Jeremy.Jean@oss.cyber.gouv.fr/ (local)
    Assisted-by: Copilot:gpt-5.6-sol
    Signed-off-by: Siddharth Chintamaneni [off-list ref]
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: [off-list ref] # <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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help