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: <8605141c-b615-4e84-9574-81e24590df48@mailbox.org>
Date: Fri, 13 Jun 2025 19:02:28 +0200
From: Marek Vasut <marek.vasut@...lbox.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>,
 Marek Vasut <marek.vasut+bmc150@...lbox.org>,
 Hans de Goede <hansg@...nel.org>
Cc: linux-iio@...r.kernel.org, Nuno Sá <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, David Lechner <dlechner@...libre.com>,
 Jonathan Cameron <jic23@...nel.org>, 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 6/13/25 5:09 PM, Andy Shevchenko wrote:
> Strange I don't see Hans in the Cc list, so added.
> Thanks for the report and patch, my comments below.
> 
> On Fri, Jun 13, 2025 at 02:45:22PM +0200, 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.
>>
>> ACPI table dump:
> 
>>          Device (BMA2)
>>          {
>>              Name (_ADR, Zero)  // _ADR: Address
>>              Name (_HID, "BOSC0200")  // _HID: Hardware ID
>>              Name (_CID, "BOSC0200")  // _CID: Compatible ID
>>              Name (_DDN, "Accelerometer")  // _DDN: DOS Device Name
>>              Name (_UID, One)  // _UID: Unique ID
>>              Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>>              {
>>                  Name (RBUF, ResourceTemplate ()
>>                  {
>>                      I2cSerialBusV2 (0x0019, ControllerInitiated, 0x00061A80,
>>                          AddressingMode7Bit, "\\_SB.PCI0.I2C0",
>>                          0x00, ResourceConsumer, , Exclusive,
>>                          )
>>                  })
>>                  Return (RBUF) /* \_SB_.PCI0.I2C0.BMA2._CRS.RBUF */
>>              }
> 
> These lines...
> 
>>              Method (ROTM, 0, NotSerialized)
>>              {
>>                  Name (SBUF, Package (0x03)
>>                  {
>>                      "0 1 0",
>>                      "1 0 0 ",
>>                      "0 0 1"
>>                  })
>>                  Return (SBUF) /* \_SB_.PCI0.I2C0.BMA2.ROTM.SBUF */
>>              }
>>
>>              Method (_STA, 0, NotSerialized)  // _STA: Status
>>              {
>>                  Return (0x0F)
>>              }
> 
> ...are irrelevant.
> 
>>          }
>> "
>>
>> Splat, collected from debian unstable, probably not very useful:
> 
> Oh my gosh, please leave only ~3-5 *important* lines out of this, or move it
> completely to the comment block (after '---' cutter line).
> 
> This is requirement written in Submitting Patches.
> 
> ...
> 
> As for the solution, are you sure the line is not wired at all?

No . It is some cheap mini-laptop , I have no schematics or any other 
info really .

Note that I am not really familiar with x86 and ACPI, so there is that.

> IIRC Hans had a broken tales where it was simply forgotten, meaning
> the Android / Windows driver simply hardcoded needed info.
> 
> If it's the case, it should be solved differently around PDx86 special quirk
> driver for the cases like this.
There are likely two issues.

First, this driver needs to handle i2c_client->irq == 0 correctly if it 
should work without IRQ line, which the driver seems to indicate that it 
does. The current crashing the kernel is not the correct way of handling 
that. That's this patch, in some form.

Second, if this laptop has some IRQ line for this chip hidden somewhere, 
then it might need a quirk of sorts, sure. Is there some way to find 
out, without taking the thing apart and poking around with a scope ?