Re: [PATCH v10 01/28] x86/fpu/xstate: Fix the state copy function to the XSTATE buffer

From: Thomas Gleixner
Date: Fri Oct 01 2021 - 08:44:49 EST


Chang,

On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote:

> Harden copy_uabi_to_xstate() so that it can handle the case where
> __raw_xsave() returns NULL.

That's an implementation detail, but does not explain why this can
happen and what this patch is about.

> This does not happen in practice today, but theoretically could happen
> in the future.

So what does the patch "fix"? When the subject says "Fix..." then I'm
expecting a bug in the code to be fixed.

There is none because the use case which can trip over this does not
exist today. You are adding it later.

Subject: .....: Prepare copy_uabi_to_xstate() to handle dynamic features

or something like that along with a reasonable explanation.

But in a later patch you add in the very same function:

> + hdr.xfeatures &= fpu->state_mask;

which prevents that already because __raw_xsave_addr() is not invoked
for the zeroed bits in hdr.xfeatures:

> if (hdr.xfeatures & mask) {
> void *dst = __raw_xsave_addr(xsave, i);

Confused.

I'm not against the change per se, but I'm not accepting changelogs
which make no sense at all. News at 11.

Thanks,

tglx