linux-kernel - Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim

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: <DF97040MFVN2.18VE8M36CAX80@gmail.com>
Date: Sat, 27 Dec 2025 12:58:42 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "David Lechner" <dlechner@...libre.com>, "Kurt Borja"
 <kuurtb@...il.com>, "Andy Shevchenko" <andriy.shevchenko@...el.com>,
 "Lars-Peter Clausen" <lars@...afoo.de>, "Michael Hennerich"
 <Michael.Hennerich@...log.com>, "Jonathan Cameron" <jic23@...nel.org>,
 "Benson Leung" <bleung@...omium.org>, "Antoniu Miclaus"
 <antoniu.miclaus@...log.com>, "Gwendal Grignou" <gwendal@...omium.org>,
 "Shrikant Raskar" <raskar.shree97@...il.com>, "Per-Daniel Olsson"
 <perdaniel.olsson@...s.com>
Cc: Nuno Sá <nuno.sa@...log.com>, "Andy Shevchenko"
 <andy@...nel.org>, "Guenter Roeck" <groeck@...omium.org>, "Jonathan
 Cameron" <Jonathan.Cameron@...wei.com>, <linux-iio@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <chrome-platform@...ts.linux.dev>
Subject: Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct()
 implementation

On Tue Dec 23, 2025 at 12:20 PM -05, David Lechner wrote:
> On 12/11/25 8:45 PM, Kurt Borja wrote:
>> In order to eventually unify the locking API, implement
>> iio_device_claim_direct() fully inline, with the use of
>> __iio_dev_mode_lock(), which takes care of sparse annotations.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@...il.com>
>> ---

...

>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index aecda887d833..76398dbfa5ca 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -664,30 +664,42 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp);
>>  
>>  void __iio_dev_mode_lock(struct iio_dev *indio_dev) __acquires(indio_dev);
>>  void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
>> -bool __iio_device_claim_direct(struct iio_dev *indio_dev);
>> -void __iio_device_release_direct(struct iio_dev *indio_dev);
>>  
>> -/*
>> - * Helper functions that allow claim and release of direct mode
>> - * in a fashion that doesn't generate many false positives from sparse.
>> - * Note this must remain static inline in the header so that sparse
>> - * can see the __acquire() marking. Revisit when sparse supports
>> - * __cond_acquires()
>
> I think we should leave a comment that says these functions need to stay
> as static inline so that sparse works. Although I'm wondering how sparse

I'll keep that part!

> could still work since __acquire() and __release() are removed here. IIRC
> it was a bit finicky about that.

Now __iio_dev_mode_{lock,unlock}() are declared with __acquires() and
__releases() tags. These are then used conditionally in
iio_device_claim_direct().

I did test every driver with `make C=2` and it seems to work in the same
way.

...

>> +/**
>> + * iio_device_release_direct - Releases claim on direct mode
>> + * @indio_dev:	the iio_dev associated with the device
>> + *
>> + * Release the claim. Device is no longer guaranteed to stay
>> + * in direct mode.
>> + *
>> + * Use with iio_device_claim_direct()
>> + */
>>  static inline void iio_device_release_direct(struct iio_dev *indio_dev)
>
> It could make more sense to make this:
>
> #define iio_device_release_direct __iio_dev_mode_unlock
>
> To make it clear that the auto cleanup functions that come later
> don't call iio_device_release_direct() but rather call
> __iio_dev_mode_unlock() directly.
>
> Otherwise, someone could be tempted to modify the iio_device_release_direct()
> function and the changes would not have an effect when auto cleanup is used.
>
> (Same applies to the release buffer mode function.)
>

Agree. I'll implement it like this.

-- 
 ~ Kurt