linux-kernel - Re: [PATCH] iio: proximity: hx9023s: use IIO_DECLARE_BUFFER_WITH

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] [day] [month] [year] [list]

Message-ID: <20250724112317.0c9b561b@jic23-huawei>
Date: Thu, 24 Jul 2025 11:23:17 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko
 <andy@...nel.org>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 Yasin Lee <yasin.lee.x@...il.com>
Subject: Re: [PATCH] iio: proximity: hx9023s: use
 IIO_DECLARE_BUFFER_WITH_TS()

On Wed, 23 Jul 2025 10:07:55 -0500
David Lechner <dlechner@...libre.com> wrote:

> On 7/22/25 5:54 PM, David Lechner wrote:
> > Use stack-allocated IIO_DECLARE_BUFFER_WITH_TS() to declare the buffer
> > that gets used with iio_push_to_buffers_with_ts().
> > 
> > We change from a struct to IIO_DECLARE_BUFFER_WITH_TS() since
> > HX9023S_CH_NUM is 5 making channels[] larger than 8 bytes and therefore
> > the timestamp is not always as the same position depending on the number
> > of channels enabled in the scan.
> > 
> > And since the data structure is not used outside of the scope of the
> > interrupt handler, the array does not need to be in the driver state
> > struct and can just be stack-allocated.
> > 
> > Signed-off-by: David Lechner <dlechner@...libre.com>
Given the type issue you fix in next patch, I'll sit on this one for now
(otherwise I'd just have fixed up the below initialization that you call out)

+CC Yasin

> > ---
> >  drivers/iio/proximity/hx9023s.c | 13 ++++---------
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> > index 33781c3147286fb3e2f022201ccf7e908d0b6b12..1203fa4bc7512ea85b55d537e2459104b52407b9 100644
> > --- a/drivers/iio/proximity/hx9023s.c
> > +++ b/drivers/iio/proximity/hx9023s.c
> > @@ -143,12 +143,6 @@ struct hx9023s_data {
> >  	unsigned long chan_in_use;
> >  	unsigned int prox_state_reg;
> >  	bool trigger_enabled;
> > -
> > -	struct {
> > -		__le16 channels[HX9023S_CH_NUM];
> > -		aligned_s64 ts;
> > -	} buffer;
> > -
> >  	/*
> >  	 * Serialize access to registers below:
> >  	 * HX9023S_PROX_INT_LOW_CFG,
> > @@ -928,6 +922,7 @@ static const struct iio_trigger_ops hx9023s_trigger_ops = {
> >  
> >  static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
> >  {
> > +	IIO_DECLARE_BUFFER_WITH_TS(__le16, channels, HX9023S_CH_NUM);  
> 
> Ooof. I remembered to zero-initialize all of the scan structs in the other
> similar patches, but forgot we need to do the same with the array to avoid
> leaking uninitialized stack to usespace.
> 
> 	IIO_DECLARE_BUFFER_WITH_TS(__le16, channels, HX9023S_CH_NUM) = { };
> 
> >  	struct iio_poll_func *pf = private;
> >  	struct iio_dev *indio_dev = pf->indio_dev;
> >  	struct hx9023s_data *data = iio_priv(indio_dev);
> > @@ -950,11 +945,11 @@ static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
> >  
> >  	iio_for_each_active_channel(indio_dev, bit) {
> >  		index = indio_dev->channels[bit].channel;
> > -		data->buffer.channels[i++] = cpu_to_le16(data->ch_data[index].diff);
> > +		channels[i++] = cpu_to_le16(data->ch_data[index].diff);
> >  	}
> >  
> > -	iio_push_to_buffers_with_ts(indio_dev, &data->buffer,
> > -				    sizeof(data->buffer), pf->timestamp);
> > +	iio_push_to_buffers_with_ts(indio_dev, channels, sizeof(channels),
> > +				    pf->timestamp);
> >  
> >  out:
> >  	iio_trigger_notify_done(indio_dev->trig);
> >