[PATCH v4 0/6] Fix potential out-of-bounds error in some drivers

[PATCH v4 0/6] Fix potential out-of-bounds error in some drivers

[Markus Burri]

Several drivers are using debugfs and follow the same pattern.

A buffer is created on the stack with a limited size to copy the given data
from user space. The copy is performed using simple_write_to_buffer.
This function limits the input according to the specified buffer size, but
it does not write a string terminator if the buffer is truncated.
Therefore, the driver adds this zero terminator afterward.
Unfortunately, the original buffer size is used as an index, which can lead
to an out-of-bounds error.

This patch set fixes this issue in all the drivers I have detected so far.
The fix is to return an error in case of an unexpectedly long buffer being
received and to use the effective written size for the zero terminator for 
consistency.

Changes in V4:
* Revert the decrement of accepted size by one character
* Added patches for drivers with the same pattern

Changes in V3:
* Decrement accepted size by one character according to feedback

Changes in V2:
* Use effective written size as index instead of original size

---
[V3] https://lore.kernel.org/lkml/20250505203830.5117-1-markus.burri@mt.com/
[V2] https://lore.kernel.org/lkml/20250505045346.29647-1-markus.burri@mt.com/
[V1] https://lore.kernel.org/lkml/20250501063240.25295-1-markus.burri@mt.com/

Markus Burri (6):
  iio: backend: fix out-of-bound write
  accel/ivpu: Use effective buffer size for zero terminator
  iio: fix potential out-of-bound write
  gpio: fix potential out-of-bound write
  powerpc/eeh: fix potential OoB
  powerpc/eeh-powernv: fix potential OoB

 arch/powerpc/kernel/eeh.c                    |  7 ++++++-
 arch/powerpc/platforms/powernv/eeh-powernv.c |  7 ++++++-
 drivers/accel/ivpu/ivpu_debugfs.c            |  2 +-
 drivers/gpio/gpio-virtuser.c                 | 12 ++++++++++--
 drivers/iio/industrialio-backend.c           |  5 ++++-
 drivers/iio/industrialio-core.c              |  5 ++++-
 6 files changed, 31 insertions(+), 7 deletions(-)


base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
-- 
2.39.5

[PATCH v4 1/6] iio: backend: fix out-of-bound write

[Markus Burri]

The buffer is set to 80 character. If a caller write more characters,
count is truncated to the max available space in "simple_write_to_buffer".
But afterwards a string terminator is written to the buffer at offset count
without boundary check. The zero termination is written OUT-OF-BOUND.

Add a check that the given buffer is smaller then the buffer to prevent.

Fixes: 035b4989211d ("iio: backend: make sure to NULL terminate stack buffer")
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
 drivers/iio/industrialio-backend.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index a43c8d1bb3d0..31fe793e345e 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -155,11 +155,14 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
 	ssize_t rc;
 	int ret;
 
+	if (count >= sizeof(buf))
+		return -ENOSPC;
+
 	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
 	if (rc < 0)
 		return rc;
 
-	buf[count] = '\0';
+	buf[rc] = '\0';
 
 	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
 
-- 
2.39.5

Re: [PATCH v4 1/6] iio: backend: fix out-of-bound write

[Jonathan Cameron]

On Thu,  8 May 2025 15:06:07 +0200
Markus Burri <markus.burri@mt.com> wrote:

> The buffer is set to 80 character. If a caller write more characters,
> count is truncated to the max available space in "simple_write_to_buffer".
> But afterwards a string terminator is written to the buffer at offset count
> without boundary check. The zero termination is written OUT-OF-BOUND.
> 
> Add a check that the given buffer is smaller then the buffer to prevent.
> 
> Fixes: 035b4989211d ("iio: backend: make sure to NULL terminate stack buffer")
> Signed-off-by: Markus Burri <markus.burri@mt.com>
I'm looking for a tag from Nuno on this one before applying.

J
> ---
>  drivers/iio/industrialio-backend.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index a43c8d1bb3d0..31fe793e345e 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -155,11 +155,14 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
>  	ssize_t rc;
>  	int ret;
>  
> +	if (count >= sizeof(buf))
> +		return -ENOSPC;
> +
>  	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
>  	if (rc < 0)
>  		return rc;
>  
> -	buf[count] = '\0';
> +	buf[rc] = '\0';
>  
>  	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
>  

Re: [PATCH v4 1/6] iio: backend: fix out-of-bound write

[Jonathan Cameron]

On Sun, 11 May 2025 15:27:07 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu,  8 May 2025 15:06:07 +0200
> Markus Burri <markus.burri@mt.com> wrote:
> 
> > The buffer is set to 80 character. If a caller write more characters,
> > count is truncated to the max available space in "simple_write_to_buffer".
> > But afterwards a string terminator is written to the buffer at offset count
> > without boundary check. The zero termination is written OUT-OF-BOUND.
> > 
> > Add a check that the given buffer is smaller then the buffer to prevent.
> > 
> > Fixes: 035b4989211d ("iio: backend: make sure to NULL terminate stack buffer")
> > Signed-off-by: Markus Burri <markus.burri@mt.com>  
> I'm looking for a tag from Nuno on this one before applying.

Please make sure to pick up tags on earlier versions. Nuno had sent a RB for
this one which I've now added.

People don't tend to look again at patches that they've already tagged so Nuno
probably didn't see my ask for a tag above.

Anyhow, now applied but will have to wait for a rebase of my fixes-togreg tree
on rc1 once available.

Thanks,

Jonathan

> 
> J
> > ---
> >  drivers/iio/industrialio-backend.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> > index a43c8d1bb3d0..31fe793e345e 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -155,11 +155,14 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
> >  	ssize_t rc;
> >  	int ret;
> >  
> > +	if (count >= sizeof(buf))
> > +		return -ENOSPC;
> > +
> >  	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
> >  	if (rc < 0)
> >  		return rc;
> >  
> > -	buf[count] = '\0';
> > +	buf[rc] = '\0';
> >  
> >  	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> >    
> 
> 

[PATCH v4 2/6] accel/ivpu: Use effective buffer size for zero terminator

[Markus Burri]

Use the effective written size instead of original size as index for zero
termination. If the input from user-space is to larger and the input is
truncated, the original size is out-of-bound.
Since there is an upfront size check here, the change is for consistency.

Signed-off-by: Markus Burri <markus.burri@mt.com>
---
 drivers/accel/ivpu/ivpu_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/ivpu/ivpu_debugfs.c b/drivers/accel/ivpu/ivpu_debugfs.c
index f0dad0c9ce33..cd24ccd20ba6 100644
--- a/drivers/accel/ivpu/ivpu_debugfs.c
+++ b/drivers/accel/ivpu/ivpu_debugfs.c
@@ -455,7 +455,7 @@ priority_bands_fops_write(struct file *file, const char __user *user_buf, size_t
 	if (ret < 0)
 		return ret;
 
-	buf[size] = '\0';
+	buf[ret] = '\0';
 	ret = sscanf(buf, "%u %u %u %u", &band, &grace_period, &process_grace_period,
 		     &process_quantum);
 	if (ret != 4)
-- 
2.39.5

Re: [PATCH v4 2/6] accel/ivpu: Use effective buffer size for zero terminator

[Jacek Lawrynowicz]

Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>

On 5/8/2025 3:06 PM, Markus Burri wrote:
> Use the effective written size instead of original size as index for zero
> termination. If the input from user-space is to larger and the input is
> truncated, the original size is out-of-bound.
> Since there is an upfront size check here, the change is for consistency.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---
>  drivers/accel/ivpu/ivpu_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_debugfs.c b/drivers/accel/ivpu/ivpu_debugfs.c
> index f0dad0c9ce33..cd24ccd20ba6 100644
> --- a/drivers/accel/ivpu/ivpu_debugfs.c
> +++ b/drivers/accel/ivpu/ivpu_debugfs.c
> @@ -455,7 +455,7 @@ priority_bands_fops_write(struct file *file, const char __user *user_buf, size_t
>  	if (ret < 0)
>  		return ret;
>  
> -	buf[size] = '\0';
> +	buf[ret] = '\0';
>  	ret = sscanf(buf, "%u %u %u %u", &band, &grace_period, &process_grace_period,
>  		     &process_quantum);
>  	if (ret != 4)

Re: [PATCH v4 2/6] accel/ivpu: Use effective buffer size for zero terminator

[Jacek Lawrynowicz]

Thanks for the fix, applied to drm-misc-fixes

On 5/8/2025 3:06 PM, Markus Burri wrote:
> Use the effective written size instead of original size as index for zero
> termination. If the input from user-space is to larger and the input is
> truncated, the original size is out-of-bound.
> Since there is an upfront size check here, the change is for consistency.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---
>  drivers/accel/ivpu/ivpu_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_debugfs.c b/drivers/accel/ivpu/ivpu_debugfs.c
> index f0dad0c9ce33..cd24ccd20ba6 100644
> --- a/drivers/accel/ivpu/ivpu_debugfs.c
> +++ b/drivers/accel/ivpu/ivpu_debugfs.c
> @@ -455,7 +455,7 @@ priority_bands_fops_write(struct file *file, const char __user *user_buf, size_t
>  	if (ret < 0)
>  		return ret;
>  
> -	buf[size] = '\0';
> +	buf[ret] = '\0';
>  	ret = sscanf(buf, "%u %u %u %u", &band, &grace_period, &process_grace_period,
>  		     &process_quantum);
>  	if (ret != 4)

[PATCH v4 3/6] iio: fix potential out-of-bound write

[Markus Burri]

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(-)

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", &reg, &val);
 
-- 
2.39.5

Re: [PATCH v4 3/6] iio: fix potential out-of-bound write

[Jonathan Cameron]

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", &reg, &val);
>  

Re: [PATCH v4 3/6] iio: fix potential out-of-bound write

[Jonathan Cameron]

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", &reg, &val);
>  

[PATCH v4 4/6] gpio: fix potential out-of-bound write

[Markus Burri]

Check that the input size does not exceed the buffer size.
If a caller write more characters, count is truncated to the max available
space in "simple_write_to_buffer".
Write a zero termination afterwards.

Signed-off-by: Markus Burri <markus.burri@mt.com>
---
 drivers/gpio/gpio-virtuser.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c
index 13407fd4f0eb..9f3c491f5af1 100644
--- a/drivers/gpio/gpio-virtuser.c
+++ b/drivers/gpio/gpio-virtuser.c
@@ -401,10 +401,15 @@ static ssize_t gpio_virtuser_direction_do_write(struct file *file,
 	char buf[32], *trimmed;
 	int ret, dir, val = 0;
 
-	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
+	if (size >= sizeof(buf))
+		return -EINVAL;
+
+	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
 	if (ret < 0)
 		return ret;
 
+	buf[ret] = '\0';
+
 	trimmed = strim(buf);
 
 	if (strcmp(trimmed, "input") == 0) {
@@ -623,12 +628,15 @@ static ssize_t gpio_virtuser_consumer_write(struct file *file,
 	char buf[GPIO_VIRTUSER_NAME_BUF_LEN + 2];
 	int ret;
 
+	if (count >= sizeof(buf))
+		return -EINVAL;
+
 	ret = simple_write_to_buffer(buf, GPIO_VIRTUSER_NAME_BUF_LEN, ppos,
 				     user_buf, count);
 	if (ret < 0)
 		return ret;
 
-	buf[strlen(buf) - 1] = '\0';
+	buf[ret] = '\0';
 
 	ret = gpiod_set_consumer_name(data->ad.desc, buf);
 	if (ret)
-- 
2.39.5

Re: [PATCH v4 4/6] gpio: fix potential out-of-bound write

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on b4432656b36e5cc1d50a1f2dc15357543add530e]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Burri/iio-backend-fix-out-of-bound-write/20250508-211644
base:   b4432656b36e5cc1d50a1f2dc15357543add530e
patch link:    https://lore.kernel.org/r/20250508130612.82270-5-markus.burri%40mt.com
patch subject: [PATCH v4 4/6] gpio: fix potential out-of-bound write
config: x86_64-buildonly-randconfig-003-20250509 (https://download.01.org/0day-ci/archive/20250509/202505091754.285hHbr2-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250509/202505091754.285hHbr2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505091754.285hHbr2-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpio/gpio-virtuser.c:404:6: error: use of undeclared identifier 'size'; did you mean 'ksize'?
     404 |         if (size >= sizeof(buf))
         |             ^~~~
         |             ksize
   include/linux/slab.h:491:8: note: 'ksize' declared here
     491 | size_t ksize(const void *objp);
         |        ^
   1 error generated.


vim +404 drivers/gpio/gpio-virtuser.c

   393	
   394	static ssize_t gpio_virtuser_direction_do_write(struct file *file,
   395							const char __user *user_buf,
   396							size_t count, loff_t *ppos,
   397							bool atomic)
   398	{
   399		struct gpio_virtuser_line_data *data = file->private_data;
   400		struct gpio_desc *desc = data->ad.desc;
   401		char buf[32], *trimmed;
   402		int ret, dir, val = 0;
   403	
 > 404		if (size >= sizeof(buf))
   405			return -EINVAL;
   406	
   407		ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
   408		if (ret < 0)
   409			return ret;
   410	
   411		buf[ret] = '\0';
   412	
   413		trimmed = strim(buf);
   414	
   415		if (strcmp(trimmed, "input") == 0) {
   416			dir = 1;
   417		} else if (strcmp(trimmed, "output-high") == 0) {
   418			dir = 0;
   419			val = 1;
   420		} else if (strcmp(trimmed, "output-low") == 0) {
   421			dir = val = 0;
   422		} else {
   423			return -EINVAL;
   424		}
   425	
   426		if (!atomic)
   427			ret = gpio_virtuser_set_direction(desc, dir, val);
   428		else
   429			ret = gpio_virtuser_set_direction_atomic(desc, dir, val);
   430		if (ret)
   431			return ret;
   432	
   433		return count;
   434	}
   435	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 4/6] gpio: fix potential out-of-bound write

[Bartosz Golaszewski]

On Thu, May 8, 2025 at 3:07 PM Markus Burri <markus.burri@mt.com> wrote:
>
> Check that the input size does not exceed the buffer size.
> If a caller write more characters, count is truncated to the max available
> space in "simple_write_to_buffer".
> Write a zero termination afterwards.
>
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---

Looks good and does fix an issue that can be easily reproduced with
KASAN enabled. Please fix the issues reported by the build bot and
resend (as a patch separate from the rest of this series). Thanks.

Bartosz

[PATCH v4 5/6] powerpc/eeh: fix potential OoB

[Markus Burri]

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>
---
 arch/powerpc/kernel/eeh.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 83fe99861eb1..92ef05d3678d 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1734,10 +1734,15 @@ static ssize_t eeh_force_recover_write(struct file *filp,
 	char buf[20];
 	int ret;
 
-	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
+	if (count >= sizeof(buf))
+		return -EINVAL;
+
+	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
 	if (!ret)
 		return -EFAULT;
 
+	buf[ret] = '\0';
+
 	/*
 	 * When PE is NULL the event is a "special" event. Rather than
 	 * recovering a specific PE it forces the EEH core to scan for failed
-- 
2.39.5

Re: [PATCH v4 5/6] powerpc/eeh: fix potential OoB

[Mahesh J Salgaonkar]

On 2025-05-08 15:06:11 Thu, Markus Burri 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>

Thanks for the fix.

Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>

Thanks,
-Mahesh.

> ---
>  arch/powerpc/kernel/eeh.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 83fe99861eb1..92ef05d3678d 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1734,10 +1734,15 @@ static ssize_t eeh_force_recover_write(struct file *filp,
>  	char buf[20];
>  	int ret;
>  
> -	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> +	if (count >= sizeof(buf))
> +		return -EINVAL;
> +
> +	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
>  	if (!ret)
>  		return -EFAULT;
>  
> +	buf[ret] = '\0';
> +
>  	/*
>  	 * When PE is NULL the event is a "special" event. Rather than
>  	 * recovering a specific PE it forces the EEH core to scan for failed
> -- 
> 2.39.5
> 
> 

-- 
Mahesh J Salgaonkar

[PATCH v4 6/6] powerpc/eeh-powernv: fix potential OoB

[Markus Burri]

The buffer is set to 50 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>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index db3370d1673c..3abee21fdd05 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -73,14 +73,19 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
 	char buf[50];
 	int ret;
 
+	if (count >= sizeof(buf))
+		return -EINVAL;
+
 	if (!eeh_ops || !eeh_ops->err_inject)
 		return -ENXIO;
 
 	/* Copy over argument buffer */
-	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
+	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
 	if (!ret)
 		return -EFAULT;
 
+	buf[ret] = '\0';
+
 	/* Retrieve parameters */
 	ret = sscanf(buf, "%x:%x:%x:%lx:%lx",
 		     &pe_no, &type, &func, &addr, &mask);
-- 
2.39.5

Re: [PATCH v4 6/6] powerpc/eeh-powernv: fix potential OoB

[Mahesh J Salgaonkar]

On 2025-05-08 15:06:12 Thu, Markus Burri wrote:
> The buffer is set to 50 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>

Looks perfect to me.

Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>

Thanks,
-Mahesh.

> ---
>  arch/powerpc/platforms/powernv/eeh-powernv.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index db3370d1673c..3abee21fdd05 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -73,14 +73,19 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
>  	char buf[50];
>  	int ret;
>  
> +	if (count >= sizeof(buf))
> +		return -EINVAL;
> +
>  	if (!eeh_ops || !eeh_ops->err_inject)
>  		return -ENXIO;
>  
>  	/* Copy over argument buffer */
> -	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> +	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
>  	if (!ret)
>  		return -EFAULT;
>  
> +	buf[ret] = '\0';
> +
>  	/* Retrieve parameters */
>  	ret = sscanf(buf, "%x:%x:%x:%lx:%lx",
>  		     &pe_no, &type, &func, &addr, &mask);
> -- 
> 2.39.5
> 
> 

-- 
Mahesh J Salgaonkar

Re: [PATCH v4 0/6] Fix potential out-of-bounds error in some drivers

[Bartosz Golaszewski]

On Thu, May 8, 2025 at 3:06 PM Markus Burri <markus.burri@mt.com> wrote:
>
> Several drivers are using debugfs and follow the same pattern.
>
> A buffer is created on the stack with a limited size to copy the given data
> from user space. The copy is performed using simple_write_to_buffer.
> This function limits the input according to the specified buffer size, but
> it does not write a string terminator if the buffer is truncated.
> Therefore, the driver adds this zero terminator afterward.
> Unfortunately, the original buffer size is used as an index, which can lead
> to an out-of-bounds error.
>
> This patch set fixes this issue in all the drivers I have detected so far.
> The fix is to return an error in case of an unexpectedly long buffer being
> received and to use the effective written size for the zero terminator for
> consistency.
>
> Changes in V4:
> * Revert the decrement of accepted size by one character
> * Added patches for drivers with the same pattern
>
> Changes in V3:
> * Decrement accepted size by one character according to feedback
>
> Changes in V2:
> * Use effective written size as index instead of original size
>
> ---
> [V3] https://lore.kernel.org/lkml/20250505203830.5117-1-markus.burri@mt.com/
> [V2] https://lore.kernel.org/lkml/20250505045346.29647-1-markus.burri@mt.com/
> [V1] https://lore.kernel.org/lkml/20250501063240.25295-1-markus.burri@mt.com/
>
> Markus Burri (6):
>   iio: backend: fix out-of-bound write
>   accel/ivpu: Use effective buffer size for zero terminator
>   iio: fix potential out-of-bound write
>   gpio: fix potential out-of-bound write
>   powerpc/eeh: fix potential OoB
>   powerpc/eeh-powernv: fix potential OoB
>

Please, don't send this as a series, it makes absolutely no sense.
These patches - other than fixing similar issues - are completely
unrelated.

Bartosz