linux-hardening - Re: [PATCH v3] ethtool: Replace 0-length array with flexible array

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: <CAMZ6RqJXnUBxqyCFRaLxELjnvGzn9NoiePV2RVwBzAZRGH_Qmg@mail.gmail.com>
Date:   Fri, 6 Jan 2023 23:25:14 +0900
From:   Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To:     Jann Horn <jannh@...gle.com>
Cc:     Kees Cook <keescook@...omium.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        kernel test robot <lkp@...el.com>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Sean Anderson <sean.anderson@...o.com>,
        Alexandru Tachici <alexandru.tachici@...log.com>,
        Amit Cohen <amcohen@...dia.com>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH v3] ethtool: Replace 0-length array with flexible array

On Fri. 6 Jan 2023 at 22:19, Jann Horn <jannh@...gle.com> wrote:
> On Fri, Jan 6, 2023 at 5:28 AM Kees Cook <keescook@...omium.org> wrote:
> > Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> > "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> > using -fstrict-flex-arrays=3:
> >
> > net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> > net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> >   558 |                         .fs.location = info->rule_locs[i],
> >       |                                        ~~~~~~~~~~~~~~~^~~
> > In file included from include/linux/ethtool.h:19,
> >                  from include/uapi/linux/ethtool_netlink.h:12,
> >                  from include/linux/ethtool_netlink.h:6,
> >                  from net/ethtool/common.c:3:
> > include/uapi/linux/ethtool.h:1186:41: note: while referencing
> > 'rule_locs'
> >  1186 |         __u32                           rule_locs[0];
> >       |                                         ^~~~~~~~~
> >
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Cc: "David S. Miller" <davem@...emloft.net>
> > Cc: Jakub Kicinski <kuba@...nel.org>
> > Cc: Andrew Lunn <andrew@...n.ch>
> > Cc: kernel test robot <lkp@...el.com>
> > Cc: Oleksij Rempel <linux@...pel-privat.de>
> > Cc: Sean Anderson <sean.anderson@...o.com>
> > Cc: Alexandru Tachici <alexandru.tachici@...log.com>
> > Cc: Amit Cohen <amcohen@...dia.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> > Cc: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> > Cc: netdev@...r.kernel.org
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> > ---
> > v3: don't use helper (vincent)
> > v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> > ---
> >  include/uapi/linux/ethtool.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 58e587ba0450..3135fa0ba9a4 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> >                 __u32                   rule_cnt;
> >                 __u32                   rss_context;
> >         };
> > -       __u32                           rule_locs[0];
> > +       __u32                           rule_locs[];
>
> Stupid question: Is this syntax allowed in UAPI headers despite not
> being part of standard C90 or C++? Are we relying on all C/C++
> compilers for pre-C99 having gcc/clang extensions?

The [0] isn't part of the C90 standard either. So having to choose
between [0] and [], the latter is the most portable nowadays.

If I do a bit of speleology, I can see that C99 flexible array members
were used as early as v2.6.19 (released in November 2006):

  https://elixir.bootlin.com/linux/v2.6.19/source/include/linux/usb/audio.h#L36

This is prior to the include/linux and include/uapi/linux split, but
believe me, this usb/audio.h file is indeed part of the uapi.
So, yes, using C99 flexible array members in the UAPI is de facto
allowed because it was used for the last 16 years.

An interesting sub question would be:

  What are the minimum compiler requirements to build a program using
the Linux UAPI?

And, after research, I could not find the answer. The requirements to
build the kernel are well documented:

  https://docs.kernel.org/process/changes.html#changes

But no clue for the uapi. I guess that at one point in 2006, people
decided that it was time to set the minimum requirement to C99. Maybe
this matches the end of life of the latest pre-C99 GCC version? The
detailed answer must be hidden somewhere on lkml.


Yours sincerely,
Vincent Mailhol