Re: [RFC PATCH v2 06/11] ptwrite uprobes: Add basic memory references

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/uprobes: missing disp32 emission for memory args causes invalid reads
--

commit a6d11e7a0c907a2ea16451bbea72e2f4d6ed70e8
Author: Andi Kleen <ak@kernel.org>

ptwrite uprobes: Add basic memory references

Add support for memory references. Currently this is only
simple cases, no indirect memory references or strings,
that would require saving/restoring registers. Only 8 and 4 byte
memory references are supported.

> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
[ ... ]
> @@ -1431,24 +1463,62 @@ int arch_uprobe_ptwrite_prepare(struct arch_uprobe *auprobe,
[ ... ]
>  	for (i = 0; i < desc->nargs; i++) {
> -		if (desc->args[i].src == UPROBE_PTW_SRC_REG) {
> +		switch (desc->args[i].src) {
[ ... ]
> +		case UPROBE_PTW_SRC_MEM: {
> +			u8 reg = desc->args[i].reg;
> +			bool wide = desc->args[i].size == 8;
> +			unsigned int arg_len = (wide ? 9 : 8) +
> +				((reg & 7) == 4) + (!wide && (reg & 8));
> +
> +			PTW_NEED(arg_len);
> +			*p++ = 0xf3;
> +			if (wide)
> +				*p++ = (reg & 8) ? 0x49 : 0x48; /* REX.W */
> +			else if (reg & 8)
> +				*p++ = 0x41; /* REX.B only (32-bit operand) */
> +			*p++ = 0x0f;
> +			*p++ = 0xae;
> +			*p++ = 0xa0 | (reg & 7); /* mod 10, reg /4, rm reg */
> +			if ((reg & 7) == 4) /* SIB escape: base rsp/esp/r12 */
> +				*p++ = 0x24;
> +			p += 4;

[Severity: High]
Does this code miss copying the displacement value for the memory access?

The stub generator in arch_uprobe_ptwrite_prepare() for memory fetch arguments
(UPROBE_PTW_SRC_MEM) configures the ptwrite instruction to use a 32-bit
displacement (mod=10) and advances the instruction pointer p by 4 bytes.
However, it appears to fail to write the requested user displacement
(desc->args[i].val) into those 4 bytes, leaving them as uninitialized zeroes.

Will the generated uprobe stub execute a memory fetch with a displacement of
0 (e.g., reading [reg + 0] instead of [reg + disp32])? This could cause the
traced application to read incorrect memory, which can lead to capturing
garbage data in the trace or crashing the application with an unintended
SIGSEGV.

> +			break;
> +		}
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917230127.924985-2-ak@kernel.org?part=6