|
From: | Richard Henderson |
Subject: | Re: [PATCH v2 23/28] target/i386: Honor xfeatures in xrstor_sigcontext |
Date: | Tue, 9 Apr 2024 08:09:17 -1000 |
User-agent: | Mozilla Thunderbird |
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~
[Prev in Thread] | Current Thread | [Next in Thread] |