linux-kernel - Re: [PATCH] x86/cpu: correct values for GDT_ENTRY

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Nov 2020 23:52:08 +0000
From:   Andrew Cooper <andrew.cooper3@...rix.com>
To:     Andy Lutomirski <luto@...nel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        X86 ML <x86@...nel.org>, Akinobu Mita <akinobu.mita@...il.com>,
        Joerg Roedel <jroedel@...e.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        <kernel-janitors@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/cpu: correct values for GDT_ENTRY_INIT

On 26/11/2020 19:15, Andy Lutomirski wrote:
> On Thu, Nov 26, 2020 at 11:07 AM Lukas Bulwahn <lukas.bulwahn@...il.com> wrote:
>> On Thu, Nov 26, 2020 at 6:16 PM Andrew Cooper <andrew.cooper3@...rix.com> wrote:
>>> On 26/11/2020 11:54, Lukas Bulwahn wrote:
>>>> Commit 1e5de18278e6 ("x86: Introduce GDT_ENTRY_INIT()") unintentionally
>>>> transformed a few 0xffff values to 0xfffff (note: five times "f" instead of
>>>> four) as part of the refactoring.
>>> The transformation in that change is correct.
>>>
>>> Segment bases are 20 bits wide in x86,

I of course meant segment limits here, rather than bases.

>>> Does:
>>>
>>> diff --git a/arch/x86/include/asm/desc_defs.h
>>> b/arch/x86/include/asm/desc_defs.h
>>> index f7e7099af595..9561f3c66e9e 100644
>>> --- a/arch/x86/include/asm/desc_defs.h
>>> +++ b/arch/x86/include/asm/desc_defs.h
>>> @@ -22,7 +22,7 @@ struct desc_struct {
>>>
>>>  #define GDT_ENTRY_INIT(flags, base, limit)                     \
>>>         {                                                       \
>>> -               .limit0         = (u16) (limit),                \
>>> +               .limit0         = (u16) (limit) & 0xFFFF,       \
>>>                 .limit1         = ((limit) >> 16) & 0x0F,       \
>>>                 .base0          = (u16) (base),                 \
>>>                 .base1          = ((base) >> 16) & 0xFF,        \
>>>
>>> fix the warning?
>>>
>> Thanks, I will try that out, and try compiling a 32-bit kernel as well.
> You should also try comparing the objdump output before and after your
> patch.  objdump -D will produce bizarre output but should work.

Expanding on this a little, if that does indeed fix the sparse warning,
then I'd make an argument for this being a bug in sparse.  Explicitly
casting to u16 is semantically and intentionally identical to & 0xffff.

~Andrew