linux-kernel - Re: [PATCH] gpiolib: Show correct direction from the beginning

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:   Thu, 20 Sep 2018 15:43:46 -0700
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
Cc:     Timur Tabi <timur@...i.org>, Stephen Boyd <swboyd@...omium.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] gpiolib: Show correct direction from the beginning

On Thu, Sep 20, 2018 at 7:14 AM Ricardo Ribalda Delgado
<ricardo.ribalda@...il.com> wrote:
> On Thu, Sep 20, 2018 at 2:20 PM Timur Tabi <timur@...i.org> wrote:

> > Users are expected to program the direction for every GPIO they want to
> > use, regardless of whatever it's set to before they open it.
>
> I do not agree that the user should program the direction of a GPIO
> which direction cannot be used.
>
> Also I am not talking about programming a gpio, I am talking about an
> technician  connecting portA to portB and burning something because
> the system provided erroneous information

So what is clear is that your need, I guess in userspace, reliable
information about the direction of the GPIOs at boot.

> > Because calling that API before properly claiming the GPIO is a
> > programming error.
>
> Is there a place where this API is defined?. Which functions require
> to be defined.? What is the correct order.?

There is nothing like such. We would have to establish semantics.
I don't see a point in it, the APIs are for using and understanding,
not for discussing API contracts. I would avoid trying to etch this
API in stone.

> > The reason why the Qualcomm driver is impacted the most is because on
> > ACPI platforms, the GPIO map is "sparse".  That is, not every GPIO
> > between 0 and n-1 actually exists.  So reading a GPIO that doesn't exist
> > is invalid.
>
> Why are we adding GPIOs that are invalid?
> If you can figure out that a GPIO is invalid when the user claims a
> gpio, you can also figure it out when the user asks the direction.

Right and that is what the (later introduced) valid_mask
can do.

Any time we call into any of the callbacks that take an offset
we should (theoretically) check the .valid_mask if active.

Since we normally check it when requesting the GPIO, we
can't request invalid GPIOs and normally the other callbacks
would only get called after that, so we are fine.

> > I'm not sure how to properly fix this, but I wonder if we need some kind
> > of late-stage initialization where gpiolib scans all the GPIOs by
> > claiming them first, reading the directions, and then releasing them.
>
> That sounds like a good compromise. Or returning
> -unconfigured / unknown
>
> is also an option.

I would just skip over anything asked off in the valid_mask.

I feel positive of a version of this patch that respect valid_mask
some way, as that should be safe for Qualcomms usecase.

Rough consensus and running code.

Yours,
Linus Walleij