linux-kernel - Re: [PATCH v3 07/21] x86/fpu/xstate: Introduce helpers to manage dynamic xstate buffers

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: <20210204131002.GA17068@zn.tnic>
Date:   Thu, 4 Feb 2021 14:10:02 +0100
From:   Borislav Petkov <bp@...e.de>
To:     "Bae, Chang Seok" <chang.seok.bae@...el.com>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, x86-ml <x86@...nel.org>,
        "Brown, Len" <len.brown@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Liu, Jing2" <jing2.liu@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 07/21] x86/fpu/xstate: Introduce helpers to manage
 dynamic xstate buffers

On Wed, Feb 03, 2021 at 04:10:24AM +0000, Bae, Chang Seok wrote:
> Okay, how about:
> “
> This alignment bit is set if the state is saved on a 64B-aligned address in
> the compacted format buffer.
> "

I'd prefer:

/*
 * True if the buffer of the corresponding XFEATURE is located on the next 64
 * byte boundary. Otherwise, it follows the preceding component immediately.
 */
static bool xstate_aligns[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = false };

> The threshold here could be more than that. But the intention is a heads-up to
> (re-)consider (a) a new allocation mechanism and (b) to shrink the memory
> allocation.
> 
> Also, the AMX state size is limited to (a bit less than) 64KB and it was
> discussed that vmalloc() will be okay with AMX [2].

So if nothing is going to grow over 64K, why are we even talking about this?

> Maybe it is possible to backtrack this allocation failure out of #NM handling.
> But the tracepoint can provide a clear context, although limited to those
> using it.

Yes, add it when it is really needed. Not slapping it proactively and
hoping for any potential usage.

> Indeed, this is the most preferred way on one hand. But there was a change to
> the current allocation approach by Ingo about 6 years ago [3].

Yah, there's that. :-\

I guess it needs to stay embedded. Oh well.

I guess you can diminish the confusion by doing this:

struct fpu {

	...

	union fpregs_state		*state;

	union fpregs_state		__default_state;
};

and tasks will have

	state = &__default_state;

set up by default in fpu__copy() etc.

AMX tasks will simply change the pointer to the vmalloc'ed xstate
buffer. This way at least the pointer will be a single one and the task
alloc code will simply reroute it instead of having two things to pay
attention to.

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg