linux-kernel - RE: [RFC PATCH 07/22] x86/fpu/xstate: Introduce helpers to manage an xstate area dynamically

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: <BYAPR11MB346378236EEB357339C0DD81E0040@BYAPR11MB3463.namprd11.prod.outlook.com>
Date:   Tue, 13 Oct 2020 22:00:16 +0000
From:   "Brown, Len" <len.brown@...el.com>
To:     Andy Lutomirski <luto@...nel.org>,
        "Bae, Chang Seok" <chang.seok.bae@...el.com>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...e.de>,
        X86 ML <x86@...nel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Liu, Jing2" <jing2.liu@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 07/22] x86/fpu/xstate: Introduce helpers to manage an
 xstate area dynamically



>
> From: Andy Lutomirski <luto@...nel.org> 
>
> > +       /*
> > +        * The caller may be under interrupt disabled condition. Ensure interrupt
> > +        * allowance before the memory allocation, which may involve with page faults.
> > +        */
> > +       local_irq_enable();

> ... you can't just enable IRQs here.  If IRQs are off, they're off for a reason.  Secondly, if they're *on*, you just forgot that fact.

Good catch.  This is a trap handler from user-space and should never be called with irqs disabled,
So the local_irq_enable() should be dead code.  Chang suggested that he erroneously left it in
from a previous implementation.

> > +       /* We need 64B aligned pointer, but vmalloc() returns a page-aligned address */
> > +       state_ptr = vmalloc(newsz);

> And allocating this state from vmalloc() seems questionable.  Why are you doing this?

While the buffer needs to be virtually contiguous, it doesn't need to be physically contiguous,
And so vmalloc() is less overhead than kmalloc() for this.

Thanks,
-Len