Re: [PATCHv3 07/12] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, May 21, 2026 at 01:21:21PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Changing the USDT_NOP macro to emit a 10-byte NOP breaks libbpf discovery, disables kernel uprobe optimizations, and relies on an architecturally unsafe patching concept.
> --

all the other changes are in other commits, I keep the changes separated
for now, we can squash them later if we think it's good idea

jirka

> 
> commit 963f950434fd00b2b52a00da58d5968fea9d4ebc
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch
> 
> This commit updates the USDT_NOP macro for x86_64 to emit a 1-byte nop
> followed by a 10-byte nop. This is intended to test new uprobe
> optimization support in the kernel.
> 
> > diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
> > --- a/tools/testing/selftests/bpf/usdt.h
> > +++ b/tools/testing/selftests/bpf/usdt.h
> [ ... ]
> > @@ -313,7 +313,7 @@ struct usdt_sema { volatile unsigned short active; };
> >  #if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
> >  #define USDT_NOP			nop 0
> >  #elif defined(__x86_64__)
> > -#define USDT_NOP                       .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
> > +#define USDT_NOP                       .byte 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 /* nop, nop10 */
> 
> [Severity: Medium]
> Does this change to a 10-byte nop break the libbpf discovery logic?
> 
> Looking at has_nop_combo() in tools/lib/bpf/usdt.c, it seems to
> explicitly check for the 6-byte sequence (0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00).
> 
> If has_nop_combo() returns false, the target IP won't advance to the
> multi-byte nop, leaving the probe at the 1-byte instruction.
> 
> Since the kernel's can_optimize() in arch/x86/kernel/uprobes.c requires
> an instruction length of 5, does this cause the selftest to silently
> degrade to the slower int3 breakpoint path rather than testing the
> intended optimization?
> 
> Additionally, is it safe to replace a 10-byte nop with a 5-byte call on
> x86_64?
> 
> The 5-byte call pushes IP + 5 to the stack. When the uprobe trampoline
> returns, execution would resume at IP + 5, which lands exactly in the
> middle of the 10-byte nop sequence.
> 
> Could those remaining 5 bytes (0x00, 0x00, 0x00, 0x00, 0x00) be
> incorrectly decoded, leading to unexpected behavior like memory
> corruption or application crashes?
> 
> Would using two consecutive 5-byte nops instead of a single 10-byte
> nop safely provide the padding needed for a 5-byte patch?
> 
> >  #else
> >  #define USDT_NOP			nop
> >  #endif
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260521124411.31133-8-jolsa@kernel.org?part=1