linux-kernel - Re: [PATCH] nospec: Move array_index_nospec parameter checking into separate macro

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] [day] [month] [year] [list]

Message-ID: <20180219115452.GC30394@arm.com>
Date:   Mon, 19 Feb 2018 11:54:53 +0000
From:   Will Deacon <will.deacon@....com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] nospec: Move array_index_nospec parameter checking into
 separate macro

Hi Geert,

On Mon, Feb 19, 2018 at 12:47:08PM +0100, Geert Uytterhoeven wrote:
> On Mon, Feb 5, 2018 at 3:16 PM, Will Deacon <will.deacon@....com> wrote:
> > For architectures providing their own implementation of
> > array_index_mask_nospec in asm/barrier.h, attempting to use WARN_ONCE to
> > complain about out-of-range parameters using WARN_ON results in a mess
> > of mutually-dependent include files.
> >
> > Rather than unpick the dependencies, simply have the core code in nospec.h
> > perform the checking for us.
> >
> > Cc: Dan Williams <dan.j.williams@...el.com>
> > Cc: Ingo Molnar <mingo@...hat.com>
> > Signed-off-by: Will Deacon <will.deacon@....com>

[...]

> > @@ -61,7 +67,7 @@ static inline unsigned long array_index_mask_nospec(unsigned long index,
> >  ({                                                                     \
> >         typeof(index) _i = (index);                                     \
> >         typeof(size) _s = (size);                                       \
> > -       unsigned long _mask = array_index_mask_nospec(_i, _s);          \
> > +       unsigned long _mask = array_index_mask_nospec_check(_i, _s);    \
> >                                                                         \
> >         BUILD_BUG_ON(sizeof(_i) > sizeof(long));                        \
> >         BUILD_BUG_ON(sizeof(_s) > sizeof(long));                        \
> 
> This change is commit 8fa80c503b484ddc ("nospec: Move array_index_nospec()
> parameter checking into separate macro") in v4.16-rc2, and triggers the
> following warning with gcc-4.1.2:
> 
>     net/wireless/nl80211.c: In function ‘parse_txq_params’:
>     net/wireless/nl80211.c:2099: warning: comparison is always false
> due to limited range of data type
> 
> Reverting the commit gets rid of the warning.

This is all getting ripped out, so stay tuned. The check is bogus, generates
crappy code and I did a poor job at macro-ising it. Apart from that, it's
great.

https://git.kernel.org/tip/1d91c1d2c80cb70e2e553845e278b87a960c04da

Will