linux-kernel - Re: [PATCH][V4] lib: fix __sysfs_match

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]

Date:   Wed, 26 Jun 2019 07:57:59 +0000
From:   "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>
To:     "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>
Subject: Re: [PATCH][V4] lib: fix __sysfs_match_string() helper when n != -1

On Tue, 2019-06-25 at 12:42 -0700, Andrew Morton wrote:
> [External]
> 
> 
> On Tue, 25 Jun 2019 16:28:12 +0300 Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> 
> > On Tue, Jun 25, 2019 at 04:01:04PM +0300, Alexandru Ardelean wrote:
> > > The documentation the `__sysfs_match_string()` helper mentions that `n`
> > > (the size of the given array) should be:
> > >  * @n: number of strings in the array or -1 for NULL terminated arrays
> > > 
> > > The behavior of the function is different, in the sense that it exits on
> > > the first NULL element in the array.
> > > 
> > > This patch changes the behavior, to exit the loop when a NULL element is
> > > found, and the size of the array is provided as -1.
> > > 
> > > All current users of __sysfs_match_string() & sysfs_match_string() provide
> > > contiguous arrays of strings, so this behavior change doesn't influence
> > > anything (at this point in time).
> > > 
> > > This behavior change allows for an array of strings to have NULL elements
> > > within the array, which will be ignored. This is particularly useful when
> > > creating mapping of strings and integers (as bitfields or other HW
> > > description).
> > 
> > Since it does nothing for current users and comes without an example,
> > it's hard to justify the need.
> 
> Presumably "split this patch away from series" means there's some code
> which uses this.  A reference to this in the changelog would be good.
> 
> > The code itself looks good to me.
> 
> Sure.  But the kerneldoc description of __sysfs_match_string() could do
> with an update.
> 

Apologies for any clumsiness here [from my side].

The series tried to handle a change for supporting gaps in IIO enums.
Particularly this patch:
https://patchwork.kernel.org/patch/10935003/

These enums use __sysfs_match_string() to match strings to enum/int values.

There were 3 versions:
https://patchwork.kernel.org/project/linux-iio/list/?series=108481  [V1]
https://patchwork.kernel.org/project/linux-iio/list/?series=108603  [V2]
https://patchwork.kernel.org/project/linux-iio/list/?series=115201  [V3]

The last series [V3] has an unresolved discussion about how to handle gaps in IIO enums.
This will be resolved at a later point in time.

In the meantime, I thought I'd suggest this change on it's own, mostly due to the kerneldoc not being accurate.

I'm fine with both fixing the kerneldoc or fixing the code.
We will still have to resolve the discussion in IIO about how to handle those gapped enums.

>