linux-kernel - Re: [PATCH] iio: accel: bmc150: Do not configure IRQ registers if no IRQ connected

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: <20250621181733.3cb6111e@jic23-huawei>
Date: Sat, 21 Jun 2025 18:17:33 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Hans de Goede <hansg@...nel.org>
Cc: Marek Vasut <marek.vasut+bmc150@...lbox.org>, linux-iio@...r.kernel.org,
 Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko
 <andy@...nel.org>, David Lechner <dlechner@...libre.com>, Julien Stephan
 <jstephan@...libre.com>, Peter Zijlstra <peterz@...radead.org>, Salvatore
 Bonaccorso <carnil@...ian.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: accel: bmc150: Do not configure IRQ registers if
 no IRQ connected

On Mon, 16 Jun 2025 14:42:54 +0200
Hans de Goede <hansg@...nel.org> wrote:

> Hi,
> 
> On 13-Jun-25 14:45, Marek Vasut wrote:
> > The BMC150 on Onemix 2S does not have IRQ line described in ACPI tables,
> > which leads to bmc150_accel_core_probe() being called with irq=0, which
> > leads to bmc150_accel_interrupts_setup() never being called, which leads
> > to struct bmc150_accel_data *data ->interrupts[i].info being left unset
> > to NULL. Later, userspace can indirectly trigger bmc150_accel_set_interrupt()
> > which depends on struct bmc150_accel_data *data ->interrupts[i].info being
> > non-NULL, and which triggers NULL pointer dereference. This is triggered
> > e.g. from iio-sensor-proxy.
> > 
> > Fix this by skipping the IRQ register configuration in case there is no
> > IRQ connected in hardware, in a manner similar to what the driver did in
> > the very first commit which added the driver.  
> 
> ...
> 
> > Fixes: 8e22f477e143 ("iio: bmc150: refactor interrupt enabling")
> > Signed-off-by: Marek Vasut <marek.vasut+bmc150@...lbox.org>
> > ---
> > Cc: "Nuno Sá" <nuno.sa@...log.com>
> > Cc: Andy Shevchenko <andy@...nel.org>
> > Cc: David Lechner <dlechner@...libre.com>
> > Cc: Jonathan Cameron <jic23@...nel.org>
> > Cc: Julien Stephan <jstephan@...libre.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: Salvatore Bonaccorso <carnil@...ian.org>
> > Cc: linux-iio@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > ---
> >  drivers/iio/accel/bmc150-accel-core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index 744a034bb8b5..1c3583ade2b4 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -550,6 +550,9 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	if (!info)
> > +		return 0;
> > +
> >  	/* map the interrupt to the appropriate pins */
> >  	ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
> >  				 (state ? info->map_bitmask : 0));  
> 
> AFAIK the proper fix would be to not register any IIO-triggers. This fix will
> avoid the problem, but userspace might still try to use non-working triggers
> which will now silently fail.
> 
> I'm not an IIO expert, but IIRC other drivers simply skip registering their triggers
> when there is no interrupt support.

Absolutely. It is annoyingly common for devices to have some or none of the interrupt
lines actually wired so drivers should not present the interfaces if they aren't.
It is acceptable for a new driver to just fail to probe if handling the device with no
interrupts is particularly complex but in general at least some functionality tends
to be easy to implement so we prefer that.

Thanks,

Jonathan

> 
> Regards,
> 
> Hans
> 
>