On 4/8/24 21:44, Paolo Bonzini wrote:
+        /*
+         * Restore the features indicated in the frame, masked by
+         * those currently enabled.  Re-check the frame size.
+         * ??? It is not clear where the kernel does this, but it
+         * is not in check_xstate_in_sigframe, and so (probably)
+         * does not fall back to fxrstor.
+         */

I think you're referring to this in __fpu_restore_sig?

         if (use_xsave()) {
                 /*
                  * Remove all UABI feature bits not set in user_xfeatures
                  * from the memory xstate header which makes the full
                  * restore below bring them into init state. This works for
                  * fx_only mode as well because that has only FP and SSE
                  * set in user_xfeatures.
                  *
                  * Preserve supervisor states!
                  */
                 u64 mask = user_xfeatures | xfeatures_mask_supervisor();

                 fpregs->xsave.header.xfeatures &= mask;
                 success = !os_xrstor_safe(fpu->fpstate,
                                           fpu_kernel_cfg.max_features);

It is not masking against the user process's xcr0, but qemu-user's xcr0
is effectively user_xfeatures (it's computed in x86_cpu_reset_hold() and
will never change afterwards since XSETBV is privileged).

No, I'm talking about verifying that the xstate_size is large enough.

In check_xstate_in_sigframe,

        if (fx_sw->magic1 != FP_XSTATE_MAGIC1 ||
            fx_sw->xstate_size < min_xstate_size ||

Check for the trivially too small case (fxregs + header).

            fx_sw->xstate_size > current->thread.fpu.fpstate->user_size ||

Check for the trivially too large case (presumably this is to catch stupidly large values, assuming garbage).

            fx_sw->xstate_size > fx_sw->extended_size)

Check for trivial mismatch between fields.

                goto setfx;

But there's a middle case: if xfeatures > 3, then xstate_size must be > 
min_xstate_size.

I know that the kernel will handle any #GP in xrstor_from_user_sigframe, but there doesn't seem to be a real check for reading garbage beyond the given size.


r~

Reply via email to