From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 3/7] x86: Temporary disable SMAP to
 legally access user pages in kernel mode
Date: Wed, 23 Apr 2014 11:34:38 +0100
Message-ID: <5357973E.5060205@citrix.com>
References: <1398263756-32093-1-git-send-email-feng.wu@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1398263756-32093-1-git-send-email-feng.wu@intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Feng Wu <feng.wu@intel.com>
Cc: kevin.tian@intel.com, ian.campbell@citrix.com, eddie.dong@intel.com, xen-devel@lists.xen.org, JBeulich@suse.com, jun.nakajima@intel.com
List-Id: xen-devel@lists.xenproject.org

On 23/04/14 15:35, Feng Wu wrote:
> Use STAC/CLAC to temporarily disable SMAP to allow legal accesses to
> user pages in kernel mode
>
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  xen/arch/x86/domain_build.c         | 3 +++
>  xen/arch/x86/usercopy.c             | 6 ++++++
>  xen/arch/x86/x86_64/compat/entry.S  | 2 ++
>  xen/arch/x86/x86_64/entry.S         | 4 ++++
>  xen/include/asm-x86/uaccess.h       | 4 ++++
>  xen/include/asm-x86/x86_64/system.h | 2 ++
>  6 files changed, 21 insertions(+)
>
> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index 84ce392..15af110 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -778,6 +778,7 @@ int __init construct_dom0(
>      }
>      bootstrap_map(NULL);
>  
> +    stac();

As constructing dom0 is trusted, this should be near the top of top of
the function

>      if ( UNSET_ADDR != parms.virt_hypercall )
>      {
>          if ( (parms.virt_hypercall < v_start) ||
> @@ -787,6 +788,7 @@ int __init construct_dom0(
>              write_ptbase(current);
>              printk("Invalid HYPERCALL_PAGE field in ELF notes.\n");
>              rc = -1;
> +            clac();

and this should be after the out label.

>              goto out;
>          }
>          hypercall_page_initialise(
> @@ -1150,6 +1152,7 @@ int __init construct_dom0(
>                 elf_check_broken(&elf));
>  
>      iommu_dom0_init(dom0);
> +    clac();

This will need rebasing given recent changes in staging.

~Andrew