linux-hardening - Re: [PATCH v2 03/10] gpiolib: implement low-level, shared GPIO support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMRc=MfG9fOFQT2L0B5gSGvgvzFTKOVKW4zXLYZ6tmVocZJwjg@mail.gmail.com>
Date: Mon, 27 Oct 2025 19:02:38 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Kees Cook <kees@...nel.org>, Mika Westerberg <westeri@...nel.org>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Linus Walleij <linus.walleij@...aro.org>, Manivannan Sadhasivam <mani@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Saravana Kannan <saravanak@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Andy Shevchenko <andy@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
	Srinivas Kandagatla <srini@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, linux-hardening@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-sound@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 03/10] gpiolib: implement low-level, shared GPIO support

On Fri, Oct 24, 2025 at 9:09 AM Andy Shevchenko
<andriy.shevchenko@...el.com> wrote:
>
> On Thu, Oct 23, 2025 at 08:55:27PM +0200, Bartosz Golaszewski wrote:
> > On Wed, Oct 22, 2025 at 7:34 PM Andy Shevchenko
> > <andriy.shevchenko@...el.com> wrote:
> > > On Wed, Oct 22, 2025 at 03:10:42PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > > +             if (!strends(prop->name, "-gpios") &&
> > > > +                 !strends(prop->name, "-gpio") &&
> > >
> > > > +                 strcmp(prop->name, "gpios") != 0 &&
> > > > +                 strcmp(prop->name, "gpio") != 0)
> > >
> > > We have gpio_suffixes for a reason (also refer to for_each_gpio_property_name()
> > > implementation, and yes I understand the difference, this is just a reference
> > > for an example of use of the existing list of suffixes).
> >
> > And how would you use them here - when you also need the hyphen -
> > without multiple dynamic allocations instead of static strings?
>
> Something like
>
>         char suffix[6];

Well that is quite fragile, isn't it? Not only does it require 7
characters but if we ever add a "-gpios+1" suffix, it will not work
correctly. At some point you just have to open-code these things for
better readability. I doubt you save any code with this.

>         bool found = false;
>
>         for_each_gpio_property_name(suffix, "")
>                 found = found || strends();
>         for_each_gpio_property_name(suffix, NULL)
>                 found = found || (strcmp() == 0);
>         if (!found)
>                 continue;
>
> Of course with more thinking this may be optimized to avoid snprintf()
> (probably with a new helper macro or so).
>
> But see my next reply, I found something more interesting.
>

I must be missing it. I don't know what you're referring to.

> ...
>
> > > > +     /* No need to dev->release() anything. */
> > >
> > > And is it okay?
> > >
> > > See drivers/base/core.c:2567
> > >
> > > WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
> >
> > Huh... you're not wrong but I haven't seen this warning. Do people
> > just use empty functions in this case?
>
> I dunno. Maybe something applies a default release in you case? Can you
> investigate that?
>

Ah, this only happens when the release is triggered, not at
registration. If I force a release, I see it alright.

Bart