linux-kernel - Re: [PATCH v8 2/2] iio: adc: Add ti-ads1018 driver

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: <bef2e71c-fd76-4e73-9e53-422f9fa96757@baylibre.com>
Date: Mon, 15 Dec 2025 12:09:31 -0600
From: David Lechner <dlechner@...libre.com>
To: Kurt Borja <kuurtb@...il.com>, Jonathan Cameron <jic23@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Tobias Sperling <tobias.sperling@...ting.com>
Cc: Nuno Sá <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v8 2/2] iio: adc: Add ti-ads1018 driver

On 12/15/25 10:54 AM, Kurt Borja wrote:
> On Mon Dec 15, 2025 at 10:55 AM -05, David Lechner wrote:
>> On 12/11/25 10:25 PM, Kurt Borja wrote:
>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>>> analog-to-digital converters.
>>>

...

>>> +static irqreturn_t ads1018_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *indio_dev = pf->indio_dev;
>>> +	struct ads1018 *ads1018 = iio_priv(indio_dev);
>>> +	struct {
>>> +		__be16 conv;
>>> +		aligned_s64 ts;
>>> +	} scan = {};
>>> +	int ret;
>>> +
>>
>>
>>> +	if (iio_device_claim_buffer_mode(indio_dev))
>>> +		goto out_notify_done;
>>
>> This should not be needed. It should not be possible to
>> exit buffer mode without triggers being stopped first.
>> (No other driver is doing this.)
> 
> Previously I had my own lock here because ads1018_spi_read_exclusive()
> needs locking.

What exactly are we protecting against here? I.e. give side-by-side
lists of possible concurrent function calls where there could be a
problem.

Any call to iio_device_claim_direct() will already fail without
calling iio_device_claim_buffer_mode() here. And since this is
an interrupt handler, we don't have to worry about reentrancy (it
can't be called again until the previous call returns). And nowhere
else in the driver is calling iio_device_claim_buffer_mode(). So
calling it here doesn't actually add any protection AFAICT.

> I dropped it per your suggestion in v1 I believe, but
> given the discussion in the cleanup.h series I was thinking in bringing
> it back.
> 
> But yes, the scope can be adjusted too.
>