linux-kernel - Re: [PATCH v9 04/20] gpio: uapi: define uAPI v2

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:   Wed, 23 Sep 2020 18:30:31 +0800
From:   Kent Gibson <warthog618@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v9 04/20] gpio: uapi: define uAPI v2

On Wed, Sep 23, 2020 at 01:04:05PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 22, 2020 at 5:34 AM Kent Gibson <warthog618@...il.com> wrote:
> >

[snip]

> > There is also some minor renaming of fields for consistency compared to
> > their v1 counterparts, e.g. offset rather than lineoffset or line_offset,
> > and consumer rather than consumer_label.
> >
> > Additionally, v1 GPIOHANDLES_MAX becomes GPIO_V2_LINES_MAX in v2 for
> > clarity, and the gpiohandle_data __u8 array becomes a bitmap in
> > gpio_v2_line_values.
> >
> > The v2 uAPI is mostly a reorganisation and extension of v1, so userspace
> > code, particularly libgpiod, should readily port to it.
> 
> ...
> 
> > +struct gpio_v2_line_config {
> > +       __aligned_u64 flags;
> > +       __u32 num_attrs;
> 
> > +       /* Pad to fill implicit padding and reserve space for future use. */
> > +       __u32 padding[5];
> 
> Probably I somehow missed the answer, but why do we need 5 here and not 1?
> 

Sorry, I got tired of repeating myself, and just acked that we disagree
on the approach here.

Your suggestion to use the size for version would result in an
explosion of ioctl signatures - every time we add a field we have to add
a new ioctl and handle it separately in gpio_ioctl() or linereq_ioctl().

Instead what we do here is reserve some space for future use - that we
can replace with fields without changing the signature.
The padding is required to be zeroed now, and any future use will take
a 0 to mean "leave alone".

The sizes are a guestimate as to what may be needed in the future, and
as such are almost certainly wrong - but hopefully on the high side.
If that fails we can always fall back to your approach.

Cheers,
Kent.