BPF List
 help / color / mirror / Atom feed
From: Saket Kumar Bhaskar <skb99@linux.ibm.com>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH v2 2/2] powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto
Date: Thu, 30 Jul 2026 20:17:37 +0530	[thread overview]
Message-ID: <amtkCdPeeO0ld_d7@linux.ibm.com> (raw)
In-Reply-To: <20260730055641.85ADA1F000E9@smtp.kernel.org>

On Thu, Jul 30, 2026 at 05:56:41AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The assembly trampoline `arch_bpf_timed_may_goto` allocates a fixed 112-byte stack frame and stores caller-saved BPF registers at offsets `32(r1)` to `72(r1)`. On PowerPC64 Big Endian (ELFv1 ABI), this writes directly into the caller's TOC save area and the callee's Parameter Save Area (PSA), causing silent stack corruption.
> --
> 
> --- Patch [2]: [PATCH v2 2/2] powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto ---
> commit fc9f3132494d759b31f127f67b0e0815b4960f74
> Author: Saket Kumar Bhaskar <skb99@linux.ibm.com>
> 
> powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto
> 
> This commit fixes a link-time failure in the powerpc64 JIT for timed may_goto
> when CONFIG_PPC_KERNEL_PCREL is enabled. It replaces a direct bl instruction
> with the CFUNC() macro to properly annotate the call and avoid TOC restoration
> linker errors.
> 
> > diff --git a/arch/powerpc/net/bpf_timed_may_goto.S b/arch/powerpc/net/bpf_timed_may_goto.S
> > index 6fd8b1c9f4ac8..84ecf6fa7f5dc 100644
> > --- a/arch/powerpc/net/bpf_timed_may_goto.S
> > +++ b/arch/powerpc/net/bpf_timed_may_goto.S
> > @@ -36,7 +36,7 @@ SYM_FUNC_START(arch_bpf_timed_may_goto)
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the stack frame allocation
> in arch_bpf_timed_may_goto violate the PowerPC64 Big Endian (ELFv1) ABI?
> 
> Before this hunk, the trampoline allocates a fixed 112-byte stack frame and
> saves caller-saved BPF registers:
> 
> 	stdu    r1, -112(r1)
> 
> 	/* Save BPF registers R0 - R5 (r8, r3-r7) */
> 	std     r3, 32(r1)
> 	std     r4, 40(r1)
> 	std     r5, 48(r1)
> 	std     r6, 56(r1)
> 	std     r7, 64(r1)
> 	std     r8, 72(r1)
> 
> Because STACK_FRAME_MIN_SIZE is exactly 112 bytes in ELFv1, writing to
> offsets 40 through 72 places the saved registers directly into the TOC
> save area (40) and the Parameter Save Area (48-72), which the ABI
> reserves for linkage stubs and the callee.
> 
> When bpf_check_timed_may_goto executes, the ELFv1 ABI permits the C
> compiler to spill its first argument into the PSA slot at 48(r1). Will
> this silently overwrite BPF register R3 (stored at 48)? Similarly, could
> linkage stubs clobber R2 by writing to the TOC save area at 40(r1)?
> 
> Since the BPF verifier assumes these registers are preserved by the
> trampoline, this register clobbering could invalidate the verifier's
> safety tracking at runtime and lead to out-of-bounds accesses.
> 
Since this is not related to the change in current patch, will fix this
in another set of patches.

-Saket
> >  	 * BPF_REG_FP is r31; BPF_REG_AX is r12 (stack offset in bytes).
> >  	 */
> >  	add     r3, r31, r12
> > -	bl      bpf_check_timed_may_goto
> > +	bl      CFUNC(bpf_check_timed_may_goto)
> >  
> >  	/* Put return value back into AX */
> >  	mr      r12, r3
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785387718.git.skb99@linux.ibm.com?part=2

  reply	other threads:[~2026-07-30 14:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1785387718.git.skb99@linux.ibm.com>
2026-07-30  5:46 ` [PATCH v2 1/2] powerpc64/bpf: Fix build break in bpf_jit_emit_func_call_rel() Saket Kumar Bhaskar
2026-07-30 13:06   ` Hari Bathini
2026-07-30  5:46 ` [PATCH v2 2/2] powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto Saket Kumar Bhaskar
2026-07-30  5:56   ` sashiko-bot
2026-07-30 14:47     ` Saket Kumar Bhaskar [this message]
2026-07-30 13:07   ` Hari Bathini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amtkCdPeeO0ld_d7@linux.ibm.com \
    --to=skb99@linux.ibm.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox