| Message ID | 20250508130612.82270-4-markus.burri@mt.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/6] iio: backend: fix out-of-bound write | expand |
On Thu, 8 May 2025 15:06:09 +0200 Markus Burri <markus.burri@mt.com> wrote: > The buffer is set to 20 characters. If a caller write more characters, > count is truncated to the max available space in "simple_write_to_buffer". > To protect from OoB access, check that the input size fit into buffer and > add a zero terminator after copy to the end of the copied data. > > Signed-off-by: Markus Burri <markus.burri@mt.com> > --- Applied to the fixes-togreg branch of iio.git. I'd still like some more eyes on this if anyone has time though as experience teaches me that subtle tweaks to string manipulation end conditions are easy places to make mistakes! I'll not be pushing out as non rebasing until I rebase on rc1 anyway so we have time. Thanks, Jonathan > drivers/iio/industrialio-core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index b9f4113ae5fc..ebf17ea5a5f9 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -410,12 +410,15 @@ static ssize_t iio_debugfs_write_reg(struct file *file, > char buf[80]; > int ret; > > + if (count >= sizeof(buf)) > + return -EINVAL; > + > ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, > count); > if (ret < 0) > return ret; > > - buf[count] = '\0'; > + buf[ret] = '\0'; > > ret = sscanf(buf, "%i %i", ®, &val); >
On Thu, 8 May 2025 15:06:09 +0200 Markus Burri <markus.burri@mt.com> wrote: > The buffer is set to 20 characters. If a caller write more characters, > count is truncated to the max available space in "simple_write_to_buffer". > To protect from OoB access, check that the input size fit into buffer and > add a zero terminator after copy to the end of the copied data. > > Signed-off-by: Markus Burri <markus.burri@mt.com> I added Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()") If it predates that we'll need a manual backport anyway. If you have time to take a look at that Markus that would be great. Jonathan > --- > drivers/iio/industrialio-core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index b9f4113ae5fc..ebf17ea5a5f9 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -410,12 +410,15 @@ static ssize_t iio_debugfs_write_reg(struct file *file, > char buf[80]; > int ret; > > + if (count >= sizeof(buf)) > + return -EINVAL; > + > ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, > count); > if (ret < 0) > return ret; > > - buf[count] = '\0'; > + buf[ret] = '\0'; > > ret = sscanf(buf, "%i %i", ®, &val); >
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index b9f4113ae5fc..ebf17ea5a5f9 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -410,12 +410,15 @@ static ssize_t iio_debugfs_write_reg(struct file *file, char buf[80]; int ret; + if (count >= sizeof(buf)) + return -EINVAL; + ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); if (ret < 0) return ret; - buf[count] = '\0'; + buf[ret] = '\0'; ret = sscanf(buf, "%i %i", ®, &val);
The buffer is set to 20 characters. If a caller write more characters, count is truncated to the max available space in "simple_write_to_buffer". To protect from OoB access, check that the input size fit into buffer and add a zero terminator after copy to the end of the copied data. Signed-off-by: Markus Burri <markus.burri@mt.com> --- drivers/iio/industrialio-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)