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: <c42243fd-ffd2-4144-875e-b156133eb031@kernel.org>
Date: Sat, 21 Jun 2025 22:14:37 +0200
From: Hans de Goede <hansg@...nel.org>
To: Marek Vasut <marek.vasut@...lbox.org>, Jonathan Cameron <jic23@...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

Hi Marek,

On 21-Jun-25 7:24 PM, Marek Vasut wrote:
> On 6/21/25 7:17 PM, Jonathan Cameron wrote:
>> 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.
> I haven't gotten to this again ... yet ... I can try and add some sort of polling fallback maybe ?

IIO has separate interfaces for exporting channels which can be polled by
userspace and for triggers which is a more event driven interface.

It should be possible to modify the driver to skip the trigger registration,
while keeping the channels. E.g. iio-sensor-proxy will then automaticallt
switch to polling in userspace.

Regards,

Hans