linux-kernel - Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

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: <CAKv+Gu-Vc-+BDGAWOLJ79KqjL7ZW5vxUk=VW8MqEJyjO2fM6NQ@mail.gmail.com>
Date:   Wed, 1 Feb 2017 19:31:21 +0000
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Joe Perches <joe@...ches.com>
Cc:     Laura Abbott <labbott@...hat.com>,
        Will Deacon <will.deacon@....com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Markus Trippelsdorf <markus@...ppelsdorf.de>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, james.greenhalgh@....com,
        Gregory Clement <gregory.clement@...e-electrons.com>,
        Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

On 1 February 2017 at 19:04, Joe Perches <joe@...ches.com> wrote:
> On Wed, 2017-02-01 at 18:19 +0000, Ard Biesheuvel wrote:
>> On 1 February 2017 at 17:36, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
>> > I still think order_base_2() is broken, since it may invoke
>> > roundup_pow_of_two() with an input value that is documented as
>> > producing undefined output. I would argue that the below is the
>> > correct fix.
>> >
>> > diff --git a/include/linux/log2.h b/include/linux/log2.h
>> > index fd7ff3d91e6a..46523731bec0 100644
>> > --- a/include/linux/log2.h
>> > +++ b/include/linux/log2.h
>> > @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>> >   *  ... and so on.
>> >   */
>> >
>> > -#define order_base_2(n) ilog2(roundup_pow_of_two(n))
>> > +static inline __attribute__((__const__))
>> > +unsigned long __order_base_2(unsigned long n)
>> > +{
>> > +       return n ? 1UL << fls_long(n - 1) : 1;
>> > +}
>> > +
>> > +#define order_base_2(n)                                \
>> > +(                                              \
>> > +       __builtin_constant_p(n) ? (             \
>> > +               ((n) < 2) ? (n) :               \
>> > +               ilog2((n) - 1) + 1) :           \
>> > +       ilog2(__order_base_2(n))                \
>> > + )
>> >
>> >  #endif /* _LINUX_LOG2_H */
>>
>> Actually, there is a still a redundant shift/fls() in there, this is
>> even simpler:
>>
>> diff --git a/include/linux/log2.h b/include/linux/log2.h
>> index fd7ff3d91e6a..4741534bd7af 100644
>> --- a/include/linux/log2.h
>> +++ b/include/linux/log2.h
>> @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>>   *  ... and so on.
>>   */
>>
>> -#define order_base_2(n) ilog2(roundup_pow_of_two(n))
>> +static inline __attribute__((__const__))
>
> commonly __attribute_const__
>

... except in <linux/ilog2.h>, which probably predates that.

>> +unsigned long __order_base_2(unsigned long n)
>> +{
>> +       return n > 1 ? ilog2(n - 1) + 1 : 0;
>> +}
>> +
>> +#define order_base_2(n)                                \
>> +(                                              \
>> +       __builtin_constant_p(n) ? (             \
>> +               ((n) < 2) ? (n) :               \
>> +               ilog2((n) - 1) + 1) :           \
>> +       __order_base_2(n)                       \
>> + )
>
> Does this work properly when n is a signed negative value?
>

No, but order_base_2() is explicitly only defined for inputs [0, ->),
so its behavior for negative inputs is best left undefined.