On 26/05/17 06:56PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] Silent truncation of 64-bit values when written to a 32-bit > register access function. > -- > > commit 64785a0c1416e2cf718b558aa7a2cc9ac39b2d81 > Author: Rodrigo Alencar <[email protected]> > > iio: core: support 64-bit register through debugfs > > This commit introduces a debugfs_reg64_access function pointer to iio_info > to allow 64-bit register reads and writes through debugfs. The file operation > callbacks are modified to prefer the 64-bit variant when available. > > > diff --git a/drivers/iio/industrialio-core.c > > b/drivers/iio/industrialio-core.c > > index e587aca79b8e7..5c8404efd0a56 100644 > > --- a/drivers/iio/industrialio-core.c > > +++ b/drivers/iio/industrialio-core.c > > [ ... ] > > > @@ -429,7 +439,7 @@ static ssize_t iio_debugfs_write_reg(struct file *file, > > > > buf[ret] = '\0'; > > > > - ret = sscanf(buf, "%i %i", ®, &val); > > + ret = sscanf(buf, "%i %lli", ®, &val64); > > > > 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); > > Could this introduce a regression where values are silently truncated if the > provided input exceeds 32 bits? > > Since the input is now unconditionally parsed as a 64-bit integer, falling > back to debugfs_reg_access (which takes an unsigned int for writeval) will > implicitly cast val64. If a user provides a value larger than UINT_MAX to a > driver that only provides the 32-bit callback, it appears the value will be > truncated rather than rejected with an error like -EINVAL.
Maybe -EINVAL can be returned for that case indeed. For a debug interface I did not see that as a problem. > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=2 -- Kind regards, Rodrigo Alencar
