Re: [Xen-devel] [PATCH v5 08/10] x86: Enable Supervisor Mode Access Prevention (SMAP) for Xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.05.14 at 12:10, <feng.wu@xxxxxxxxx> wrote:
> Supervisor Mode Access Prevention (SMAP) is a new security
> feature disclosed by Intel, please refer to the following
> document:
> 
> http://software.intel.com/sites/default/files/319433-014.pdf 
> 
> If CR4.SMAP = 1, supervisor-mode data accesses are not allowed
> to linear addresses that are accessible in user mode. If CPL < 3,
> SMAP protections are disabled if EFLAGS.AC = 1. If CPL = 3, SMAP
> applies to all supervisor-mode data accesses (these are implicit
> supervisor accesses) regardless of the value of EFLAGS.AC.
> 
> This patch enables SMAP in Xen to prevent Xen hypervisor from
> accessing pv guest data, whose translation paging-structure
> entries' U/S flags are all set.
> 
> Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

however with the request to work on the comment below.

> @@ -1386,6 +1395,23 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>                 initrdidx);
>  
>      /*
> +     * Temporarily clear SMAP bit in CR4 to allow the user-page accesses in
> +     * construct_dom0(). This looks a little hackish, but using stac()/clac()
> +     * to do this is not so proper here, since in construct_dom0(),
> +     * copy_from_user() is call, so we cannot make construct_dom0() wrapped
> +     * by stac()/clac() as a whole, or the AC bit will remain cleared after
> +     * copy_from_user() in construct_dom0(), this will induce problems.
> +     *
> +     * On the other hand, if we call stac()/clac() in construct_dom0(), we
> +     * must carefully handle some concer cases, since there are many branches
> +     * in it.
> +     *
> +     * So we clear SMAP before construct_dom0() and set it back after it.
> +     */

Proposal:

+     * Temporarily clear SMAP bit in CR4 to allow user-page accesses in
+     * construct_dom0(). This looks a little hackish, but using stac()/clac()
+     * to do this is not appropriate here: Since in construct_dom0() calls
+     * copy_from_user(), so we cannot wrap construct_dom0() as a whole in
+     * stac()/clac() - the AC bit remaining cleared after copy_from_user()
+     * would induce problems in construct_dom0().
+     *
+     * On the other hand, if we used stac()/clac() in construct_dom0(), we
+     * would need to carefully handle some corner cases.
+     *
+     * So we clear SMAP before construct_dom0() and enable it back afterwards.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel