linux-kernel - Re: [PATCH] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aQCHt9JL0Bc4Pduv@smile.fi.intel.com>
Date: Tue, 28 Oct 2025 11:07:03 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Miaoqian Lin <linmq006@...il.com>, Markus Burri <markus.burri@...com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>,
	Angelo Dureghello <adureghello@...libre.com>,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] iio: dac: ad3552r-hs: fix out-of-bound write in
 ad3552r_hs_write_data_source

On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
> On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
> > On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:

+Cc: Markus Burri for the da9374819eb3

...

> > > +	if (count >= sizeof(buf))
> > > +		return -ENOSPC;
> > 
> > But this makes the validation too strict now.
> > 
> > >  	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> > >  				     count);
> > 
> > You definitely failed to read the code that implements the above.
> > 
> > >  	if (ret < 0)
> > >  		return ret;
> 
> > > -	buf[count] = '\0';
> > > +	buf[ret] = '\0';
> 
> Maybe this line is what we might need, but I haven't checked deeper if it's a
> problem.

So, copy_to_user() and copy_from_user() are always inlined macros.
The simple_write_to_buffer() is not. The question here is how
the __builit_object_size() will behave on the address given as a parameter to
copy_from_user() in simple_write_to_buffer().

If it may detect reliably that the buffer is the size it has. I believe it's
easy for the byte arrays on stack.

That said, without proof that compiler is unable to determine the destination
buffer size, this patch and the one by Markus are simple noise which actually
changes an error code on the overflow condition.

The only line that assigns NUL character might be useful in some cases
(definitely when buffer comes through indirect calls from a heap, etc).

> > NAK.
> > 
> > This patch is an unneeded churn.

-- 
With Best Regards,
Andy Shevchenko