[PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Jonathan Cameron]

From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
to userspace if the data passed in is not a multiple of 8 bytes and the
timestamp is enabled. Specify __GFP_ZERO for the devm_krealloc()
to ensure any extra space is cleared.

Fixes: 95ec3fdf2b79 ("iio: core: Introduce iio_push_to_buffers_with_ts_unaligned()")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260529121005.1470-1-kimjinseob88%40gmail.com
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
---

v1: https://patchwork.kernel.org/project/linux-iio/patch/20260604084307.640053-1-jic23@kernel.org/

Drop the memset in favour of __GFP_ZERO.  (Andy)
Thanks to Nuno who pointed out there is a self test to ensure this
behaves as expected.

 drivers/iio/industrialio-buffer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 531fc4ccc15d..4960c8377932 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2464,7 +2464,8 @@ int iio_push_to_buffers_with_ts_unaligned(struct iio_dev *indio_dev,
 
 		bb = devm_krealloc(&indio_dev->dev,
 				   iio_dev_opaque->bounce_buffer,
-				   indio_dev->scan_bytes, GFP_KERNEL);
+				   indio_dev->scan_bytes,
+				   GFP_KERNEL | __GFP_ZERO);
 		if (!bb)
 			return -ENOMEM;
 		iio_dev_opaque->bounce_buffer = bb;
-- 
2.55.0

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Nuno Sá]

On Mon, 2026-08-03 at 02:22 +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> 
> iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
> to userspace if the data passed in is not a multiple of 8 bytes and the
> timestamp is enabled. Specify __GFP_ZERO for the devm_krealloc()
> to ensure any extra space is cleared.
> 
> Fixes: 95ec3fdf2b79 ("iio: core: Introduce iio_push_to_buffers_with_ts_unaligned()")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260529121005.1470-1-kimjinseob88%40gmail.com
> Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> 
> v1: https://patchwork.kernel.org/project/linux-iio/patch/20260604084307.640053-1-jic23@kernel.org/
> 
> Drop the memset in favour of __GFP_ZERO.  (Andy)
> Thanks to Nuno who pointed out there is a self test to ensure this
> behaves as expected.
> 
>  drivers/iio/industrialio-buffer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 531fc4ccc15d..4960c8377932 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -2464,7 +2464,8 @@ int iio_push_to_buffers_with_ts_unaligned(struct iio_dev *indio_dev,
>  
>  		bb = devm_krealloc(&indio_dev->dev,
>  				   iio_dev_opaque->bounce_buffer,
> -				   indio_dev->scan_bytes, GFP_KERNEL);
> +				   indio_dev->scan_bytes,
> +				   GFP_KERNEL | __GFP_ZERO);
>  		if (!bb)
>  			return -ENOMEM;
>  		iio_dev_opaque->bounce_buffer = bb;

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Joshua Crofts]

On Mon,  3 Aug 2026 02:22:24 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> 
> iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
> to userspace if the data passed in is not a multiple of 8 bytes and the
> timestamp is enabled. Specify __GFP_ZERO for the devm_krealloc()
> to ensure any extra space is cleared.
> 
> Fixes: 95ec3fdf2b79 ("iio: core: Introduce iio_push_to_buffers_with_ts_unaligned()")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260529121005.1470-1-kimjinseob88%40gmail.com
> Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> ---

LGTM, but shouldn't this be a V3 as the previous patch was a V2?
I got confused for a bit...

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards,
Joshua Crofts

[PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Jonathan Cameron]

iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
to userspace if the data passed in is not a multiple of 8 bytes and the
timestamp is enabled.  Use memset() to zero it after resizing.

Fixes: 95ec3fdf2b79 ("iio: core: Introduce iio_push_to_buffers_with_ts_unaligned()")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260529121005.1470-1-kimjinseob88%40gmail.com
Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
v2: Zero the new buffer not the old one (which might not exist)
    Drop bonus Reported-by:
---
 drivers/iio/industrialio-buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5c3df993bea2..157b0415091a 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2445,6 +2445,7 @@ int iio_push_to_buffers_with_ts_unaligned(struct iio_dev *indio_dev,
 				   indio_dev->scan_bytes, GFP_KERNEL);
 		if (!bb)
 			return -ENOMEM;
+		memset(bb, 0, indio_dev->scan_bytes);
 		iio_dev_opaque->bounce_buffer = bb;
 		iio_dev_opaque->bounce_buffer_size = indio_dev->scan_bytes;
 	}
-- 
2.54.0

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Andy Shevchenko]

On Thu, Jun 04, 2026 at 09:43:07AM +0100, Jonathan Cameron wrote:
> iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
> to userspace if the data passed in is not a multiple of 8 bytes and the
> timestamp is enabled.  Use memset() to zero it after resizing.

...

>  				   indio_dev->scan_bytes, GFP_KERNEL);
>  		if (!bb)
>  			return -ENOMEM;
> +		memset(bb, 0, indio_dev->scan_bytes);

May I suggest different approach, id est use __GFP_ZERO instead of hunting
correct pointers?

With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Jonathan Cameron]

On Thu, 4 Jun 2026 12:10:42 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Jun 04, 2026 at 09:43:07AM +0100, Jonathan Cameron wrote:
> > iio_push_to_buffers_with_ts_unaligned() leaks uninitialized heap memory
> > to userspace if the data passed in is not a multiple of 8 bytes and the
> > timestamp is enabled.  Use memset() to zero it after resizing.  
> 
> ...
> 
> >  				   indio_dev->scan_bytes, GFP_KERNEL);
> >  		if (!bb)
> >  			return -ENOMEM;
> > +		memset(bb, 0, indio_dev->scan_bytes);  
> 
> May I suggest different approach, id est use __GFP_ZERO instead of hunting
> correct pointers?

That's a weird beast when combined with a krealloc so I was a bit
nervous about readability (and less so whether it was correct).
It should be fine in that we will either get stale data or zeros
because we always use this path to allocate the buffer so if you
think it is obviously fine then I don't mind.

The oddities are that if an object grows within a slab but doesn't need
a new one we are relying on those bits happening to be zero based on the
original allocation doing __GFP_ZERO as well (as it's the same call)

I'm nervous though as that region off the end is sometimes used for
debug objects and I really don't understand that bit of slab well enough.
If it actually does this, then seems like we'd end up with a lot of
nasty corner cases so I assume it doesn't.  However, I couldn't convince
myself enough not to just force a memset of the whole thing.

> 
> With Best Regards,
> Andy Shevchenko
> 
> 
> 

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Andy Shevchenko]

On Fri, Jun 05, 2026 at 02:50:00PM +0100, Jonathan Cameron wrote:
> On Thu, 4 Jun 2026 12:10:42 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Jun 04, 2026 at 09:43:07AM +0100, Jonathan Cameron wrote:

...

> > >  				   indio_dev->scan_bytes, GFP_KERNEL);
> > >  		if (!bb)
> > >  			return -ENOMEM;
> > > +		memset(bb, 0, indio_dev->scan_bytes);  
> > 
> > May I suggest different approach, id est use __GFP_ZERO instead of hunting
> > correct pointers?
> 
> That's a weird beast when combined with a krealloc so I was a bit
> nervous about readability (and less so whether it was correct).
> It should be fine in that we will either get stale data or zeros
> because we always use this path to allocate the buffer so if you
> think it is obviously fine then I don't mind.
> 
> The oddities are that if an object grows within a slab but doesn't need
> a new one we are relying on those bits happening to be zero based on the
> original allocation doing __GFP_ZERO as well (as it's the same call)
> 
> I'm nervous though as that region off the end is sometimes used for
> debug objects and I really don't understand that bit of slab well enough.
> If it actually does this, then seems like we'd end up with a lot of
> nasty corner cases so I assume it doesn't.

Such a bug will be a serious issue in mm. I don't think it exists.
But, of course, chances are not completely 0.

Current users (not a comprehensive list)

arch/arm64/kvm/nested.c:92
drivers/firmware/efi/capsule-loader.c:61
drivers/gpio/gpiolib.c:5198
drivers/media/platform/amphion/vdec.c:329
drivers/net/ethernet/intel/ice/ice_lib.c:3044
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c:785
drivers/net/wireless/ti/wl18xx/main.c:1582
drivers/nvme/target/pci-epf.c:708
drivers/spi/spi.c:1411
drivers/usb/gadget/function/uvc_configfs.c:880
fs/ceph/mdsmap.c:322
fs/smb/server/ksmbd_work.c:123
lib/tests/slub_kunit.c:273
sound/soc/meson/meson-card-utils.c:49

Also note the kernel-doc for krealloc_array(). It has some note WRT __GFP_ZERO
and it means the flag must be supported. Also there is a note in mm/slub.c.

> However, I couldn't convince myself enough not to just force a memset of the
> whole thing.

I personally would go with __GFP_ZERO.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Nuno Sá]

On Fri, 2026-06-05 at 22:18 +0300, Andy Shevchenko wrote:
> On Fri, Jun 05, 2026 at 02:50:00PM +0100, Jonathan Cameron wrote:
> > On Thu, 4 Jun 2026 12:10:42 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > On Thu, Jun 04, 2026 at 09:43:07AM +0100, Jonathan Cameron wrote:
> 
> ...
> 
> > > >  				   indio_dev->scan_bytes, GFP_KERNEL);
> > > >  		if (!bb)
> > > >  			return -ENOMEM;
> > > > +		memset(bb, 0, indio_dev->scan_bytes);  
> > > 
> > > May I suggest different approach, id est use __GFP_ZERO instead of hunting
> > > correct pointers?
> > 
> > That's a weird beast when combined with a krealloc so I was a bit
> > nervous about readability (and less so whether it was correct).
> > It should be fine in that we will either get stale data or zeros
> > because we always use this path to allocate the buffer so if you
> > think it is obviously fine then I don't mind.
> > 
> > The oddities are that if an object grows within a slab but doesn't need
> > a new one we are relying on those bits happening to be zero based on the
> > original allocation doing __GFP_ZERO as well (as it's the same call)
> > 
> > I'm nervous though as that region off the end is sometimes used for
> > debug objects and I really don't understand that bit of slab well enough.
> > If it actually does this, then seems like we'd end up with a lot of
> > nasty corner cases so I assume it doesn't.
> 
> Such a bug will be a serious issue in mm. I don't think it exists.
> But, of course, chances are not completely 0.
> 
> Current users (not a comprehensive list)
> 
> arch/arm64/kvm/nested.c:92
> drivers/firmware/efi/capsule-loader.c:61
> drivers/gpio/gpiolib.c:5198
> drivers/media/platform/amphion/vdec.c:329
> drivers/net/ethernet/intel/ice/ice_lib.c:3044
> drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c:785
> drivers/net/wireless/ti/wl18xx/main.c:1582
> drivers/nvme/target/pci-epf.c:708
> drivers/spi/spi.c:1411
> drivers/usb/gadget/function/uvc_configfs.c:880
> fs/ceph/mdsmap.c:322
> fs/smb/server/ksmbd_work.c:123
> lib/tests/slub_kunit.c:273
> sound/soc/meson/meson-card-utils.c:49
> 
> Also note the kernel-doc for krealloc_array(). It has some note WRT __GFP_ZERO
> and it means the flag must be supported. Also there is a note in mm/slub.c.
> 
> > However, I couldn't convince myself enough not to just force a memset of the
> > whole thing.
> 
> I personally would go with __GFP_ZERO.

If I'm not missing anything, this should also be proof that the flag should be fine:

https://elixir.bootlin.com/linux/v7.1-rc7/source/lib/tests/slub_kunit.c#L275

- Nuno Sá

Re: [PATCH v2] iio: buffer: Ensure bounce buffer used for unaligned case is zeroed.

[Jonathan Cameron]

On Mon, 08 Jun 2026 21:00:30 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Fri, 2026-06-05 at 22:18 +0300, Andy Shevchenko wrote:
> > On Fri, Jun 05, 2026 at 02:50:00PM +0100, Jonathan Cameron wrote:  
> > > On Thu, 4 Jun 2026 12:10:42 +0300
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > > On Thu, Jun 04, 2026 at 09:43:07AM +0100, Jonathan Cameron wrote:  
> > 
> > ...
> >   
> > > > >  				   indio_dev->scan_bytes, GFP_KERNEL);
> > > > >  		if (!bb)
> > > > >  			return -ENOMEM;
> > > > > +		memset(bb, 0, indio_dev->scan_bytes);    
> > > > 
> > > > May I suggest different approach, id est use __GFP_ZERO instead of hunting
> > > > correct pointers?  
> > > 
> > > That's a weird beast when combined with a krealloc so I was a bit
> > > nervous about readability (and less so whether it was correct).
> > > It should be fine in that we will either get stale data or zeros
> > > because we always use this path to allocate the buffer so if you
> > > think it is obviously fine then I don't mind.
> > > 
> > > The oddities are that if an object grows within a slab but doesn't need
> > > a new one we are relying on those bits happening to be zero based on the
> > > original allocation doing __GFP_ZERO as well (as it's the same call)
> > > 
> > > I'm nervous though as that region off the end is sometimes used for
> > > debug objects and I really don't understand that bit of slab well enough.
> > > If it actually does this, then seems like we'd end up with a lot of
> > > nasty corner cases so I assume it doesn't.  
> > 
> > Such a bug will be a serious issue in mm. I don't think it exists.
> > But, of course, chances are not completely 0.
> > 
> > Current users (not a comprehensive list)
> > 
> > arch/arm64/kvm/nested.c:92
> > drivers/firmware/efi/capsule-loader.c:61
> > drivers/gpio/gpiolib.c:5198
> > drivers/media/platform/amphion/vdec.c:329
> > drivers/net/ethernet/intel/ice/ice_lib.c:3044
> > drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c:785
> > drivers/net/wireless/ti/wl18xx/main.c:1582
> > drivers/nvme/target/pci-epf.c:708
> > drivers/spi/spi.c:1411
> > drivers/usb/gadget/function/uvc_configfs.c:880
> > fs/ceph/mdsmap.c:322
> > fs/smb/server/ksmbd_work.c:123
> > lib/tests/slub_kunit.c:273
> > sound/soc/meson/meson-card-utils.c:49
> > 
> > Also note the kernel-doc for krealloc_array(). It has some note WRT __GFP_ZERO
> > and it means the flag must be supported. Also there is a note in mm/slub.c.
> >   
> > > However, I couldn't convince myself enough not to just force a memset of the
> > > whole thing.  
> > 
> > I personally would go with __GFP_ZERO.  
> 
> If I'm not missing anything, this should also be proof that the flag should be fine:
> 
> https://elixir.bootlin.com/linux/v7.1-rc7/source/lib/tests/slub_kunit.c#L275
> 
Perfect!  Thanks. I'll hopefully spin an updated version in next few days

Jonathan

> - Nuno Sá