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 <[email protected]> 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/[email protected]?part=6
