linux-kernel - RE: [PATCH] gpio: Document the 'valid

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:
 <TY3PR01MB11346EC54C8672C4D28F931F686CC2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Fri, 28 Feb 2025 10:28:21 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>, Linus Walleij
	<linus.walleij@...aro.org>
CC: Matti Vaittinen <mazziesaccount@...il.com>, Matti Vaittinen
	<matti.vaittinen@...rohmeurope.com>, Bartosz Golaszewski <brgl@...ev.pl>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] gpio: Document the 'valid_mask' being internal

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@...ux-m68k.org>
> Sent: 28 February 2025 09:32
> Subject: Re: [PATCH] gpio: Document the 'valid_mask' being internal
> 
> Hi Linus,
> 
> CC Biju
> 
> On Fri, 28 Feb 2025 at 09:07, Linus Walleij <linus.walleij@...aro.org> wrote:
> > On Wed, Feb 26, 2025 at 12:42 PM Matti Vaittinen
> > <mazziesaccount@...il.com> wrote:
> > > On 26/02/2025 12:18, Linus Walleij wrote:
> > > > That's easy to check with some git grep valid_mask
> > >
> > > True. I just tried. It seems mostly Ok, but...
> > > For example the drivers/gpio/gpio-rcar.c uses the contents of the
> > > 'valid_mask' in it's set_multiple callback to disallow setting the
> > > value of masked GPIOs.
> > >
> > > For uneducated person like me, it feels this check should be done
> > > and enforced by the gpiolib and not left for untrustworthy driver
> > > writers like me! (I am working on BD79124 driver and it didn't occur
> > > to me I should check for the valid_mask in driver :) If gpiolib may
> > > call the driver's set_multiple() with masked lines - then the
> > > bd79124 driver just had one unknown bug less :rolleyes:) )
> >
> > Yeah that should be done in gpiolib.
> >
> > And I think it is, gpiolib will not allow you to request a line that
> > is not valid AFAIK.
> 
> Correct, since commit 3789f5acb9bbe088 ("gpiolib: Avoid calling
> chip->request() for unused gpios") by Biju.
> 
> > This check in rcar is just overzealous and can probably be removed.
> > Geert what do you say?
> 
> I looked at the history, and the related discussion.  It was actually Biju who added the valid_mask
> check to gpio_rcar_set_multiple() (triggering the creation of commit 3789f5acb9bbe088), and I just
> copied that when adding gpio_rcar_get_multiple().
> His v2[1] had checks in both the .request() and .set_multiple() callbacks, but it's possible he added
> the latter first, and didn't realize that became unneeded after adding the former.  The final version
> v3[2] retained only the check in .set_multiple(), as by that time the common gpiod_request_commit()
> had gained a check.
> 
> While .set_multiple() takes hardware offsets, not gpio_desc pointers, these do originate from an array
> of gpio_desc pointers, so all of them must have been requested properly.
> We never exposed set_multiple() with raw GPIO numbers to users, right?
> So I agree the check is probably not needed.
> 

I agree, when the code is mainlined at that time set_multiple() has some draw backs and hence
the check is added to take care of GPIO holes.

Cheers,
Biju