| Message ID | 20250218-dev-iio-misc-v1-1-bf72b20a1eb8@analog.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iio: small fixes and improvements | expand |
On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote: > From: Nuno Sá <nuno.sa@analog.com> > > Make sure to NULL terminate the buffer in > iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a > stack variable so we should not assume it will 0 initialized. > > Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface") > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > --- > drivers/iio/industrialio-backend.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c > index d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232c071e4 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file, > ssize_t rc; > int ret; > > - rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, count); > + rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); > if (rc < 0) > return rc; > > + buf[count] = '\0'; Does this need to be count++? Later we return count. > + > ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val); > > switch (ret) { >
On Tue, 2025-02-18 at 09:52 -0600, David Lechner wrote: > On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote: > > From: Nuno Sá <nuno.sa@analog.com> > > > > Make sure to NULL terminate the buffer in > > iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a > > stack variable so we should not assume it will 0 initialized. > > > > Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface") > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > --- > > drivers/iio/industrialio-backend.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > backend.c > > index > > d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232 > > c071e4 100644 > > --- a/drivers/iio/industrialio-backend.c > > +++ b/drivers/iio/industrialio-backend.c > > @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct > > file *file, > > ssize_t rc; > > int ret; > > > > - rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, > > count); > > + rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, > > count); > > if (rc < 0) > > return rc; > > > > + buf[count] = '\0'; > > Does this need to be count++? Later we return count. > Don't think so... count comes down from userspace. The termination is local so we do not want to return count + 1 when userspace only requested to write count. Same deal as in iio_debugfs_write_reg() Also note that we pass sizeof(buf) - 1 into simple_write_to_buffer() - Nuno Sá >
On 2/18/25 10:36 AM, Nuno Sá wrote: > On Tue, 2025-02-18 at 09:52 -0600, David Lechner wrote: >> On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote: >>> From: Nuno Sá <nuno.sa@analog.com> >>> >>> Make sure to NULL terminate the buffer in >>> iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a >>> stack variable so we should not assume it will 0 initialized. >>> >>> Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface") >>> Signed-off-by: Nuno Sá <nuno.sa@analog.com> >>> --- >>> drivers/iio/industrialio-backend.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- >>> backend.c >>> index >>> d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232 >>> c071e4 100644 >>> --- a/drivers/iio/industrialio-backend.c >>> +++ b/drivers/iio/industrialio-backend.c >>> @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct >>> file *file, >>> ssize_t rc; >>> int ret; >>> >>> - rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, >>> count); >>> + rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, >>> count); >>> if (rc < 0) >>> return rc; >>> >>> + buf[count] = '\0'; >> >> Does this need to be count++? Later we return count. >> > > Don't think so... count comes down from userspace. The termination is local so > we do not want to return count + 1 when userspace only requested to write count. > Same deal as in iio_debugfs_write_reg() > > Also note that we pass sizeof(buf) - 1 into simple_write_to_buffer() > > - Nuno Sá >> Ah, right. I get it now. Wasn't thinking so clear earlier.
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232c071e4 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file, ssize_t rc; int ret; - rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, count); + rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); if (rc < 0) return rc; + buf[count] = '\0'; + ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val); switch (ret) {