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

[sashiko-bot@kernel.org]

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.

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