[PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[Dan Carpenter]

If the *ppos value is non-zero then the simple_write_to_buffer() function
won't initialized the start of the buf[] buffer.  Non-zero values for
*ppos won't work here generally, so just test for them and return -EINVAL
at the start of the function.

Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 drivers/iio/dac/ad3552r-hs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index a9578afa7015..6bc64f53bce9 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -549,7 +549,7 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
 
 	guard(mutex)(&st->lock);
 
-	if (count >= sizeof(buf))
+	if (*ppos != 0 || count >= sizeof(buf))
 		return -ENOSPC;
 
 	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
-- 
2.53.0

Re: [PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[Angelo Dureghello]

Hi Dan,

On Mon, May 25, 2026 at 10:15:46AM +0300, Dan Carpenter wrote:
> If the *ppos value is non-zero then the simple_write_to_buffer() function
> won't initialized the start of the buf[] buffer.  Non-zero values for
> *ppos won't work here generally, so just test for them and return -EINVAL
> at the start of the function.
>
> Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
>  drivers/iio/dac/ad3552r-hs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index a9578afa7015..6bc64f53bce9 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -549,7 +549,7 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
>
>  	guard(mutex)(&st->lock);
>
> -	if (count >= sizeof(buf))
> +	if (*ppos != 0 || count >= sizeof(buf))
>  		return -ENOSPC;
>
>  	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> --

thanks for the fix,

Reviewed-by: Angelo Dureghello <adureghello@baylibre.com>

it looks like this may fix an additional overflow in case ppos is near the
end of buf[].
But i am actually missing the use case when ppos is non zero, can this
happen from shell "echo" ? Or some fseek/dd is needed to trigger it ?

Thanks,
regards,
angelo



> 2.53.0
>

Re: [PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[Dan Carpenter]

On Mon, May 25, 2026 at 01:11:42AM -0700, Angelo Dureghello wrote:
> Hi Dan,
> 
> On Mon, May 25, 2026 at 10:15:46AM +0300, Dan Carpenter wrote:
> > If the *ppos value is non-zero then the simple_write_to_buffer() function
> > won't initialized the start of the buf[] buffer.  Non-zero values for
> > *ppos won't work here generally, so just test for them and return -EINVAL
> > at the start of the function.
> >
> > Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> >  drivers/iio/dac/ad3552r-hs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > index a9578afa7015..6bc64f53bce9 100644
> > --- a/drivers/iio/dac/ad3552r-hs.c
> > +++ b/drivers/iio/dac/ad3552r-hs.c
> > @@ -549,7 +549,7 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
> >
> >  	guard(mutex)(&st->lock);
> >
> > -	if (count >= sizeof(buf))
> > +	if (*ppos != 0 || count >= sizeof(buf))
> >  		return -ENOSPC;
> >
> >  	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> > --
> 
> thanks for the fix,
> 
> Reviewed-by: Angelo Dureghello <adureghello@baylibre.com>
> 
> it looks like this may fix an additional overflow in case ppos is near the
> end of buf[].

simple_write_to_buffer() won't overflow.  It takes ppos into consideration
properly.


> But i am actually missing the use case when ppos is non zero, can this
> happen from shell "echo" ? Or some fseek/dd is needed to trigger it ?

I've never actually tried to do this.  I think lseek() or fseek() would
work.  But also the simple_write_to_buffer() function itself updates
*ppos so multiple writes would do the trick as well.

regards,
dan carpenter

Re: [PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[Maxwell Doose]

Hi Dan,

On Mon, May 25, 2026 at 2:16 AM Dan Carpenter <error27@gmail.com> wrote:
>
> If the *ppos value is non-zero then the simple_write_to_buffer() function
> won't initialized the start of the buf[] buffer.  Non-zero values for
> *ppos won't work here generally, so just test for them and return -EINVAL
> at the start of the function.
>

Please see my other email, you're also returning -ENOSPC instead of
-EINVAL here...

>
> Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
>  drivers/iio/dac/ad3552r-hs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index a9578afa7015..6bc64f53bce9 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -549,7 +549,7 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
>
>         guard(mutex)(&st->lock);
>
> -       if (count >= sizeof(buf))
> +       if (*ppos != 0 || count >= sizeof(buf))
>                 return -ENOSPC;
>

See above comment.

best regards,
max



>         ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> --
> 2.53.0
>
>

Re: [PATCH] iio: dac: ad3552r-hs: fix uninitialized data ni ad3552r_hs_write_data_source()

[Jonathan Cameron]

On Mon, 25 May 2026 08:21:51 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> Hi Dan,
> 
> On Mon, May 25, 2026 at 2:16 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > If the *ppos value is non-zero then the simple_write_to_buffer() function
> > won't initialized the start of the buf[] buffer.  Non-zero values for
> > *ppos won't work here generally, so just test for them and return -EINVAL
> > at the start of the function.
> >  
> 
> Please see my other email, you're also returning -ENOSPC instead of
> -EINVAL here...
> 
Good spot. Tweaked whilst applying.

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks

Jonathan
> >
> > Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> >  drivers/iio/dac/ad3552r-hs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> > index a9578afa7015..6bc64f53bce9 100644
> > --- a/drivers/iio/dac/ad3552r-hs.c
> > +++ b/drivers/iio/dac/ad3552r-hs.c
> > @@ -549,7 +549,7 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
> >
> >         guard(mutex)(&st->lock);
> >
> > -       if (count >= sizeof(buf))
> > +       if (*ppos != 0 || count >= sizeof(buf))
> >                 return -ENOSPC;
> >  
> 
> See above comment.
> 
> best regards,
> max
> 
> 
> 
> >         ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> > --
> > 2.53.0
> >
> >