Re: [PATCHv4 06/13] libbpf: Change has_nop_combo to work on top of nop10

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, May 26, 2026 at 09:28:39PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The patch invalidly replaces `nop5` optimization support with `nop10`, which cannot be optimized by the kernel, breaks backward compatibility for existing binaries, and causes selftest failures.
> --
> 
> commit df13b5dc7675e7ea56d8d1775d22295b93e92ab0
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> libbpf: Change has_nop_combo to work on top of nop10
> 
> This commit updates the USDT nop combo detection to expect a 10-byte nop
> instead of a 5-byte nop. It updates has_nop_combo() to match the new byte
> sequence.
> 
> > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> > index e3710933fd52a..484a4354e82ba 100644
> > --- a/tools/lib/bpf/usdt.c
> > +++ b/tools/lib/bpf/usdt.c
> [ ... ]
> > @@ -596,14 +596,14 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
> >  #if defined(__x86_64__)
> >  static bool has_nop_combo(int fd, long off)
> >  {
> > -	unsigned char nop_combo[6] = {
> > -		0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 /* nop,nop5 */
> > +	unsigned char nop_combo[11] = {
> > +		0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00,
> >  	};
> > -	unsigned char buf[6];
> > +	unsigned char buf[11];
> >  
> > -	if (pread(fd, buf, 6, off) != 6)
> > +	if (pread(fd, buf, 11, off) != 11)
> >  		return false;
> > -	return memcmp(buf, nop_combo, 6) == 0;
> > +	return memcmp(buf, nop_combo, 11) == 0;
> >  }
> 
> [Severity: High]
> Does replacing the 5-byte nop detection with a 10-byte nop detection break the
> kernel's uprobe optimization logic?
> 
> Looking at the kernel's can_optimize() in arch/x86/kernel/uprobes.c, it
> strictly requires the replaced instruction to be exactly 5 bytes. A 10-byte
> instruction fails this length check, causing the kernel to silently abort
> optimization and fall back to an unoptimized int3 probe.

this seems wrong, the related kernel change is part of the patchset

jirka

> 
> Additionally, replacing this entirely rather than supplementing it means
> libbpf no longer recognizes the nop5 combo in existing binaries. This causes
> existing USDTs to fall back to unoptimized probes and breaks BPF selftests
> that expect the +1 offset applied.
> 
> Finally, if the kernel ever updated can_optimize() to allow 10-byte nops,
> sys_uprobe() currently hardcodes a return adjustment of regs->ip += 5.
> Returning +5 bytes into a 10-byte instruction would resume execution in the
> middle of the nop sequence, which could cause an application crash.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260526205840.173790-7-jolsa@kernel.org?part=1