Re: [PATCH RFC v4 02/10] iio: core: support 64-bit register through debugfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Uninitialized stack memory read when file offset is non-zero
- [Medium] Missing newline character in debugfs read output
- [Medium] Silent truncation of 64-bit values for 32-bit registers
- [Low] Format specifier type mismatch in sscanf
--

commit 2208ef2b9ad567bd166f4ac44a53f42c899c021b
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: core: support 64-bit register through debugfs

This patch adds a debugfs_reg64_access function pointer to iio_info to
support 64-bit registers. It also updates the read and write debugfs file
operations to use the 64-bit variant when available, falling back to 32-bit.

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index bd6f4f9f45330..cb4e2ade25c0e 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
[ ... ]
> @@ -403,7 +412,7 @@ static ssize_t iio_debugfs_read_reg(struct file *file, char __user *userbuf,
>  
>  	iio_dev_opaque->read_buf_len = snprintf(iio_dev_opaque->read_buf,
>  						sizeof(iio_dev_opaque->read_buf),
> -						"0x%X\n", val);
> +						"0x%llX", val64);

Does this inadvertently drop the newline character?

[ ... ]
> @@ -429,7 +439,7 @@ static ssize_t iio_debugfs_write_reg(struct file *file,
>  
>  	buf[ret] = '\0';

Could buf be read uninitialized here if *ppos > 0?

fs/libfs.c:simple_write_to_buffer() writes user data to buf + *ppos and
returns the number of bytes written:

	res = copy_from_user(to + pos, from, count);
	if (res == count)
		return -EFAULT;
	count -= res;
	*ppos = pos + count;
	return count;

If *ppos > 0, the first *ppos bytes of buf remain uninitialized. The
subsequent sscanf() reads starting from buf[0], parsing up to count bytes
of uninitialized kernel stack memory.

>  
> -	ret = sscanf(buf, "%i %i", &reg, &val);
> +	ret = sscanf(buf, "%i %lli", &reg, &val64);

Can this trigger compiler warnings? val64 is of type u64, but %lli is the
specifier for a signed long long pointer.

>  
>  	switch (ret) {
>  	case 1:
> @@ -437,8 +447,12 @@ static ssize_t iio_debugfs_write_reg(struct file *file,
>  		break;
>  	case 2:
>  		iio_dev_opaque->cached_reg_addr = reg;
> -		ret = indio_dev->info->debugfs_reg_access(indio_dev, reg,
> -							  val, NULL);
> +		if (indio_dev->info->debugfs_reg64_access)
> +			ret = indio_dev->info->debugfs_reg64_access(indio_dev, reg,
> +								    val64, NULL);
> +		else
> +			ret = indio_dev->info->debugfs_reg_access(indio_dev, reg,
> +								  val64, NULL);

Does this silently truncate 64-bit values for 32-bit registers?

If a user provides a 64-bit value but the driver only implements the 32-bit
debugfs_reg_access callback, val64 is passed directly as the unsigned int
argument. This discards the upper 32 bits without returning an error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-ad9910-iio-driver-v4-0-d26bfd20ee3d@analog.com?part=2