linux-kernel - Re: [PATCH v2 21/23] gpio: protect the pointer to gpio_chip in gpio

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] [day] [month] [year] [list]

Message-ID: <ZcI3jkga8yfI6Nx8@smile.fi.intel.com>
Date: Tue, 6 Feb 2024 15:43:42 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>,
	Kent Gibson <warthog618@...il.com>, Alex Elder <elder@...aro.org>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	"Paul E . McKenney" <paulmck@...nel.org>,
	Wolfram Sang <wsa@...-dreams.de>, linux-gpio@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 21/23] gpio: protect the pointer to gpio_chip in
 gpio_device with SRCU

On Tue, Feb 06, 2024 at 02:23:35PM +0100, Bartosz Golaszewski wrote:
> On Tue, Feb 6, 2024 at 2:13 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
> > On Tue, Feb 06, 2024 at 01:57:39PM +0100, Bartosz Golaszewski wrote:
> > > On Tue, Feb 6, 2024 at 1:24 PM Andy Shevchenko
> > > <andriy.shevchenko@...ux.intel.com> wrote:
> > > > On Mon, Feb 05, 2024 at 08:36:39PM +0100, Bartosz Golaszewski wrote:
> > > > > On Mon, Feb 5, 2024 at 1:31 PM Andy Shevchenko
> > > > > <andriy.shevchenko@...ux.intel.com> wrote:

..

> > > > > > >  int gpiod_get_direction(struct gpio_desc *desc)
> > > > > > >  {
> > > > > > > -     struct gpio_chip *gc;
> > > > > > >       unsigned long flags;
> > > > > > >       unsigned int offset;
> > > > > > >       int ret;
> > > > > > >
> > > > > > > -     gc = gpiod_to_chip(desc);
> > > > > > > +     if (!desc)
> > > > > > > +             /* Sane default is INPUT. */
> > > > > > > +             return 1;
> > > > > >
> > > > > > Hmm... I can't imagine how this value may anyhow be used / useful.
> > > > >
> > > > > What else would you return for an optional (NULL) GPIO?
> > > >
> > > > An error. If somebody asks for direction of the non-existing GPIO, there is no
> > > > (valid) answer for that.
> >
> > > All other functions return 0 for desc == NULL to accommodate
> > > gpiod_get_optional(). I think we should stay consistent here.
> >
> > The way you proposed is inconsistent, i.e. you may not return any direction
> > for the unknown / non-existing GPIO. You speculate it will be 1, I may consider
> > that in my (hypothetical for now) case it should be 0.
> >
> > Just don't make all bananas to be oranges. It won't work.
> 
> I don't have a strong conviction here. May make it an error as well.
> It's still inconsistent though - calling gpiod_direction_output(NULL);
> will return 0 and then you get an error when you do
> gpiod_get_direction(NULL). I don't have a good solution though.

Yes, and this is the best what we can have. Because the real code may rely on
the returned value and they should be really aware on the returned values in
some cases.

-- 
With Best Regards,
Andy Shevchenko