Messages in this thread | | | Subject | Re: [PATCH v5 14/28] x86/fpu/xstate: Prevent unauthorised use of dynamic user state | From | Dave Hansen <> | Date | Tue, 29 Jun 2021 11:50:56 -0700 |
| |
On 6/29/21 11:35 AM, Bae, Chang Seok wrote: > if (likely(use_xsave())) { > + /* > + * MSR IA32_XFD write follows after this XSAVE(S). So if a > + * state component is in use, XFD should not be armed for > + * current. But, for potential changes in the future, > + * cross-check XINUSE and XFD values. If a XINUSE state > + * is XFD-armed, the following XSAVE(S) does not save the > + * state. > + * > + * Reference the shadow XFD value instead of reading the > + * MSR. > + */ > + if (xfd_capable() && boot_cpu_has(X86_FEATURE_XGETBV1)) { > + u64 current_xfd = (fpu->state_mask & xfd_capable()) ^ xfd_capable(); > + > + WARN_ON_FPU(xgetbv(1) & current_xfd); > + }
The code looks fine. But, as usual, I hate the comment. Maybe:
/* * If XFD is armed for an xfeature, XSAVE* will not save * its state. Ensure XFD is clear for all features that * are in use before XSAVE*. */
BTW, the ->state_mask calculation is a little confusing to me. I understand that fpu->state_mask shouldn't have any bits set that are unset in xgetbv(1).
This code seems to be asking the question: Are any dynamic features in their init state *and* XFD-armed?
Is it actually important to make sure that they are dynamic features? Is there *any* case where a feature (dynamic or not) can have XFD armed and be out of its init state?
| |