linux-kernel - Re: [PATCH v2] iio: common: st_sensors: Fix use of uninitialize device structs

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: <d764e4b3-9c9b-4fbf-8257-be0afd43df58@gocontroll.com>
Date: Fri, 23 May 2025 08:36:43 +0200
From: Maud Spierings <maudspierings@...ontroll.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Maud Spierings via B4 Relay <devnull+maudspierings.gocontroll.com@...nel.org>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner
 <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, Christian Heusel <christian@...sel.eu>,
 Linus Walleij <linus.walleij@...aro.org>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org,
 gregkh@...uxfoundation.org, dsimic@...jaro.com
Subject: Re: [PATCH v2] iio: common: st_sensors: Fix use of uninitialize
 device structs

On 5/22/25 19:12, Jonathan Cameron wrote:
> On Thu, 22 May 2025 13:18:55 +0200
> Maud Spierings via B4 Relay <devnull+maudspierings.gocontroll.com@...nel.org> wrote:
> 
>> From: Maud Spierings <maudspierings@...ontroll.com>
>>
>> Throughout the various probe functions &indio_dev->dev is used before it
>> is initialized. This caused a kernel panic in st_sensors_power_enable
>> when the call to devm_regulator_bulk_get_enable() fails and then calls
>> dev_err_probe() with the uninitialized device.
> Hi Maud,
> 
> 
> Curious.  Given the device_initialize() is in the allocation function
> it isn't immediately obvious that something needed might not have been initialized.
> Any idea what is being accessed in there that fails? (i.e. any idea if my
> shallow detective work found it ;)

It is indeed what you describe below, in the stack trace from my initial 
bug report it can indeed be seen that the panic happens in 
device_set_deferred_probe_reason(), __device_set_deferred_probe_reason() 
to be specific.

> https://elixir.bootlin.com/linux/v6.14.7/source/drivers/iio/industrialio-core.c#L1675
> in iio_device_alloc() is where device_initialize() is called.
> 
> Using the device in the iio_dev structure is almost certainly wrong
> but I'm surprised it crashes... So I had a quick dig.
> 
> The only path that isn't a simple print that I can spot
> is device_set_deferred_reason()  That accesses dev->p (device private
> data)  Which is initialized only in device_add().
> 
> I wonder if we should harden device_set_deferred_reason()
> against p == NULL, perhaps with a message strongly advising against
> using it with a device that hasn't been added?
> 
> Being in error paths this is the sort of subtle bug that
> rarely rears it's head :(
> 
> +CC Greg and Dragan for thoughts before anyone spins a patch.
> 
> This change is good either way. I'm just on wrong computer to pick it up
> right now.

I think I may need to send a v3, I didn't add the stable cc in my commit 
message above the SoB tag which the kernel test bot is informing me about.