Re: [RFC v1 09/19] ptwrite uprobes: Factor file-backed instruction reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] copy_insn() unconditionally returns -EIO for partial reads, breaking uprobes placed near the end of an executable.
--

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

ptwrite uprobes: Factor file-backed instruction reads

This commit refactors copy_insn() into a more generic uprobe_copy_from_file()
function. The existing copy_insn() is updated to use this generic version,
which is intended to be used by later patches.

> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 23202df2b51ae..20fa16ed85198 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
[ ... ]
> +int uprobe_copy_from_file(struct inode *inode, struct file *file,
> +			  loff_t offset, void *buf, int size)
>  {
[ ... ]
> +	while (copied < size) {
> +		file_size = i_size_read(inode);
> +		if (offset >= file_size)
>  			break;
> 
> -		len = min_t(int, size, PAGE_SIZE - (offs & ~PAGE_MASK));
> -		err = __copy_insn(mapping, filp, insn, len, offs);
> +		len = min_t(loff_t, size - copied, file_size - offset);
[ ... ]
> +static int copy_insn(struct uprobe *uprobe, struct file *filp)
> +{
> +	int ret;
> +
> +	ret = uprobe_copy_from_file(uprobe->inode, filp, uprobe->offset,
> +				   &uprobe->arch.insn,
> +				   sizeof(uprobe->arch.insn));
> +	return ret < 0 ? ret : ret == sizeof(uprobe->arch.insn) ? 0 : -EIO;

[Severity: High]
Does this unconditionally return -EIO for partial reads near the end of the
file?

When registering a uprobe on a target file where the instruction offset is
close to the end of the file (where file_size - offset is smaller than
sizeof(uprobe->arch.insn)), uprobe_copy_from_file() will stop at the file
boundary and return a byte count strictly less than the requested size.

The original copy_insn() loop stopped reading at EOF but still returned 0 as
long as no explicit error occurred, allowing partial reads to succeed.

By enforcing a strict ret == sizeof(...) check here, will this fail uprobe
registration for valid instructions that happen to reside near the end of small
binaries or scripts?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831150651.1134594-1-ak@kernel.org?part=9