linux-kernel - Re: [RFC PATCH] gpio: uapi: v2 proposal

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: <CACRpkdbputuoHFWL_FhUNR-ZywvJt=qYdaa+i2cLt-Odmgxe2w@mail.gmail.com>
Date:   Mon, 25 May 2020 10:39:42 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Kent Gibson <warthog618@...il.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [RFC PATCH] gpio: uapi: v2 proposal

On Sat, May 16, 2020 at 8:45 AM Kent Gibson <warthog618@...il.com> wrote:

> Add a new version of the uAPI to address existing 32/64bit alignment
> issues, add support for debounce, and provide some future proofing by
> adding padding reserved for future use.
>
> Signed-off-by: Kent Gibson <warthog618@...il.com>

I don't see any major problems with it, just some comments:

+ * @padding: reserved for future use and should be zero filled

*MUST* be zerofilled is what it should say.

> +struct gpioline_config {
> +       __u8 default_values[GPIOLINES_MAX];

So 32 bytes

> +       __u32 flags;
> +       __u8 direction;
> +       __u8 drive;
> +       __u8 bias;
> +       __u8 edge_detection;

Some comment in the struc that this adds up to 32 bits?

> +       __u32 debounce;
> +       __u32 padding[7]; /* for future use */

What we need to do inside the kernel with all of these
is to ascertain that they are 0 for now when they are
sent in and refuse them otherwise, I think it was Lars-Peter
Clausen who said that they had to retire a whole slew of
ioctl()s because some userspace just used unallocated
memory for this and since that was random bytes it needed to
be supported with that content forever and the reserved
padding could never be used for the reserved purpose.

This kind of comment goes for all the structs.

But mostly it is a question about the kernel code receiving
or emitting these.

Yours,
Linus Walleij