x86/pagewalk: Honor SMAP_CHECK_DISABLED
RFC PATCH 1/1 x86/pagewalk: Honor SMAP_CHECK_DISABLED
commit 4c5d78a10dc89427140a50a1df5a0b8e9f073e82 (x86/pagewalk:
Re-implement the pagetable walker) removed honoring the smap_check_policy of the running VCPU. guest_walk_tables is used by copy_{to,from}_guest for HVMs, so it is called when the hypervisor is copying data and SMAP is inappropriate to enforce.
The out-of-tree v4v hypercall copies a domain's source buffer into a different domain's destination ring. For an HVM, the kernel makes the hypercall from ring 0, so the userspace buffer access looks like a SMAP violation. In Xen 4.6, v4v could set SMAP_CHECK_DISABLED to avoid this SMAP failure, but that no longer works since the re-write.
---
commit 31ae587e6f0181bf1f7d196fe1b49357c8922e60 (x86/hvm: always do SMAP check when updating runstate_guest(v)) added smap_check_policy originally. It contained SMAP_CHECK_ENABLED, SMAP_CHECK_DISABLED, and SMAP_CHECK_HONOR_CPL_AC. SMAP_CHECK_HONOR_CPL_AC is the default and conditionalized the SMAP check on the CPL and EFLAGS.AC. SMAP_CHECK_ENABLED always turned on the SMAP check.
guest_walk_tables no longer has a CPL check. This seems to be set by emulation code through the PFEC_user_mode or PFEC_implicit walk arguments. Also with the re-write, the EFLAGS.AC check is always enforced. So update_runstate_area and update_secondary_system_time may no longer need the smap policy change? SMAP_CHECK_HONOR_CPL_AC could probably be dropped as well if that is the case.
xen/arch/x86/mm/guest_walk.c | 1 + 1 file changed, 1 insertion(+)
xen/arch/x86/mm/guest_walk.c +1 -0
@@ -416,6 +416,7 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, goto out;if ( !(walk & PFEC_insn_fetch) &&guest_smap_enabled (v) && + v->arch.smap_check_policy !=SMAP_CHECK_DISABLED && ((walk & PFEC_implicit) || !(guest_cpu_user_regs ()->eflags &X86_EFLAGS_AC )) )/* User data access and smap? Fail. */
Signature · 7 lines
2.14.3 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 6 lines · Jason Andryuk
commit
4c5d78a10dc89427140a50a1df5a0b8e9f073e82(x86/pagewalk:
Re-implement the pagetable walker) removed honoring thesmap_check_policyof the running VCPU.guest_walk_tablesis used by copy_{to,from}_guest for HVMs, so it is called when the hypervisor is copying data and SMAP is inappropriate to enforce.
Quoted 7 lines
The out-of-tree v4v hypercall copies a domain's source buffer into a different domain's destination ring. For an HVM, the kernel makes the hypercall from ring 0, so the userspace buffer access looks like a SMAP violation. In Xen 4.6, v4v could set
SMAP_CHECK_DISABLEDto avoid this SMAP failure, but that no longer works since the re-write.Signed-off-by Jason Andryuk <[email protected]>
I'm sorry, but no. It is never appropriate to ignore the guest paging settings. The correct fix here is in the kernel, to surround the v4v hypercall handler with stac/clac to whitelist userspace accesses. See the implementation of the privcmd hypercall which already does this.
If I could go back in time and nack the introduction of smap_check_policy, I would. As it stands, I'm (slowly) removing its use, and will eventually delete it.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 13 lines · Andrew Cooper
Quoted 12 lines · Jason Andryuk
commit
4c5d78a10dc89427140a50a1df5a0b8e9f073e82(x86/pagewalk:
Re-implement the pagetable walker) removed honoring thesmap_check_policyof the running VCPU.guest_walk_tablesis used by copy_{to,from}_guest for HVMs, so it is called when the hypervisor is copying data and SMAP is inappropriate to enforce.The out-of-tree v4v hypercall copies a domain's source buffer into a different domain's destination ring. For an HVM, the kernel makes the hypercall from ring 0, so the userspace buffer access looks like a SMAP violation. In Xen 4.6, v4v could set
SMAP_CHECK_DISABLEDto avoid this SMAP failure, but that no longer works since the re-write.
Quoted 6 lines
Quoted 1 line
Signed-off-by Jason Andryuk <[email protected]>I'm sorry, but no. It is never appropriate to ignore the guest paging settings. The correct fix here is in the kernel, to surround the v4v hypercall handler with stac/clac to whitelist userspace accesses. See the implementation of the privcmd hypercall which already does this.
Oh, I didn't realize stac/clac are already used with a hypercall. Thanks for the pointer.
Quoted 3 lines
If I could go back in time and nack the introduction of
smap_check_policy, I would. As it stands, I'm (slowly) removing its use, and will eventually delete it.
I think you are close. It seems to me smap_check_policy is set but not used.
Regards,
Jason
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 19 lines · Jason Andryuk
Quoted 13 lines · Andrew Cooper
Quoted 12 lines · Jason Andryuk
commit
4c5d78a10dc89427140a50a1df5a0b8e9f073e82(x86/pagewalk:
Re-implement the pagetable walker) removed honoring thesmap_check_policyof the running VCPU.guest_walk_tablesis used by copy_{to,from}_guest for HVMs, so it is called when the hypervisor is copying data and SMAP is inappropriate to enforce.The out-of-tree v4v hypercall copies a domain's source buffer into a different domain's destination ring. For an HVM, the kernel makes the hypercall from ring 0, so the userspace buffer access looks like a SMAP violation. In Xen 4.6, v4v could set
SMAP_CHECK_DISABLEDto avoid this SMAP failure, but that no longer works since the re-write.Quoted 5 lines
Quoted 1 line
Signed-off-by Jason Andryuk <[email protected]>I'm sorry, but no. It is never appropriate to ignore the guest paging settings. The correct fix here is in the kernel, to surround the v4v hypercall handler with stac/clac to whitelist userspace accesses. See the implementation of the privcmd hypercall which already does this.
Quoted 7 lines
Oh, I didn't realize stac/clac are already used with a hypercall. Thanks for the pointer.
Quoted 3 lines
If I could go back in time and nack the introduction of
smap_check_policy, I would. As it stands, I'm (slowly) removing its use, and will eventually delete it.I think you are close. It seems to me
smap_check_policyis set but not used.
So it is! Patch incomming.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel