lkml.org 
[lkml]   [2021]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[tip: x86/sgx] x86/sgx: Move provisioning device creation out of SGX driver
    The following commit has been merged into the x86/sgx branch of tip:

    Commit-ID: b3754e5d3da320af2bebb7a690002685c7f5c15c
    Gitweb: https://git.kernel.org/tip/b3754e5d3da320af2bebb7a690002685c7f5c15c
    Author: Sean Christopherson <sean.j.christopherson@intel.com>
    AuthorDate: Fri, 19 Mar 2021 20:23:09 +13:00
    Committer: Borislav Petkov <bp@suse.de>
    CommitterDate: Tue, 06 Apr 2021 19:18:46 +02:00

    x86/sgx: Move provisioning device creation out of SGX driver

    And extract sgx_set_attribute() out of sgx_ioc_enclave_provision() and
    export it as symbol for KVM to use.

    The provisioning key is sensitive. The SGX driver only allows to create
    an enclave which can access the provisioning key when the enclave
    creator has permission to open /dev/sgx_provision. It should apply to
    a VM as well, as the provisioning key is platform-specific, thus an
    unrestricted VM can also potentially compromise the provisioning key.

    Move the provisioning device creation out of sgx_drv_init() to
    sgx_init() as a preparation for adding SGX virtualization support,
    so that even if the SGX driver is not enabled due to flexible launch
    control not being available, SGX virtualization can still be enabled,
    and use it to restrict a VM's capability of being able to access the
    provisioning key.

    [ bp: Massage commit message. ]

    Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
    Signed-off-by: Kai Huang <kai.huang@intel.com>
    Signed-off-by: Borislav Petkov <bp@suse.de>
    Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
    Acked-by: Dave Hansen <dave.hansen@intel.com>
    Link: https://lkml.kernel.org/r/0f4d044d621561f26d5f4ef73e8dc6cd18cc7e79.1616136308.git.kai.huang@intel.com
    ---
    arch/x86/include/asm/sgx.h | 3 ++-
    arch/x86/kernel/cpu/sgx/driver.c | 17 +---------
    arch/x86/kernel/cpu/sgx/ioctl.c | 16 +--------
    arch/x86/kernel/cpu/sgx/main.c | 57 ++++++++++++++++++++++++++++++-
    4 files changed, 61 insertions(+), 32 deletions(-)

    diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
    index 954042e..a16e2c9 100644
    --- a/arch/x86/include/asm/sgx.h
    +++ b/arch/x86/include/asm/sgx.h
    @@ -372,4 +372,7 @@ int sgx_virt_einit(void __user *sigstruct, void __user *token,
    void __user *secs, u64 *lepubkeyhash, int *trapnr);
    #endif

    +int sgx_set_attribute(unsigned long *allowed_attributes,
    + unsigned int attribute_fd);
    +
    #endif /* _ASM_X86_SGX_H */
    diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
    index 8ce6d83..aa9b8b8 100644
    --- a/arch/x86/kernel/cpu/sgx/driver.c
    +++ b/arch/x86/kernel/cpu/sgx/driver.c
    @@ -136,10 +136,6 @@ static const struct file_operations sgx_encl_fops = {
    .get_unmapped_area = sgx_get_unmapped_area,
    };

    -const struct file_operations sgx_provision_fops = {
    - .owner = THIS_MODULE,
    -};
    -
    static struct miscdevice sgx_dev_enclave = {
    .minor = MISC_DYNAMIC_MINOR,
    .name = "sgx_enclave",
    @@ -147,13 +143,6 @@ static struct miscdevice sgx_dev_enclave = {
    .fops = &sgx_encl_fops,
    };

    -static struct miscdevice sgx_dev_provision = {
    - .minor = MISC_DYNAMIC_MINOR,
    - .name = "sgx_provision",
    - .nodename = "sgx_provision",
    - .fops = &sgx_provision_fops,
    -};
    -
    int __init sgx_drv_init(void)
    {
    unsigned int eax, ebx, ecx, edx;
    @@ -187,11 +176,5 @@ int __init sgx_drv_init(void)
    if (ret)
    return ret;

    - ret = misc_register(&sgx_dev_provision);
    - if (ret) {
    - misc_deregister(&sgx_dev_enclave);
    - return ret;
    - }
    -
    return 0;
    }
    diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
    index 7be9c06..83df20e 100644
    --- a/arch/x86/kernel/cpu/sgx/ioctl.c
    +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
    @@ -2,6 +2,7 @@
    /* Copyright(c) 2016-20 Intel Corporation. */

    #include <asm/mman.h>
    +#include <asm/sgx.h>
    #include <linux/mman.h>
    #include <linux/delay.h>
    #include <linux/file.h>
    @@ -666,24 +667,11 @@ out:
    static long sgx_ioc_enclave_provision(struct sgx_encl *encl, void __user *arg)
    {
    struct sgx_enclave_provision params;
    - struct file *file;

    if (copy_from_user(&params, arg, sizeof(params)))
    return -EFAULT;

    - file = fget(params.fd);
    - if (!file)
    - return -EINVAL;
    -
    - if (file->f_op != &sgx_provision_fops) {
    - fput(file);
    - return -EINVAL;
    - }
    -
    - encl->attributes_mask |= SGX_ATTR_PROVISIONKEY;
    -
    - fput(file);
    - return 0;
    + return sgx_set_attribute(&encl->attributes_mask, params.fd);
    }

    long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
    diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
    index 227f1e2..92cb11d 100644
    --- a/arch/x86/kernel/cpu/sgx/main.c
    +++ b/arch/x86/kernel/cpu/sgx/main.c
    @@ -1,14 +1,17 @@
    // SPDX-License-Identifier: GPL-2.0
    /* Copyright(c) 2016-20 Intel Corporation. */

    +#include <linux/file.h>
    #include <linux/freezer.h>
    #include <linux/highmem.h>
    #include <linux/kthread.h>
    +#include <linux/miscdevice.h>
    #include <linux/pagemap.h>
    #include <linux/ratelimit.h>
    #include <linux/sched/mm.h>
    #include <linux/sched/signal.h>
    #include <linux/slab.h>
    +#include <asm/sgx.h>
    #include "driver.h"
    #include "encl.h"
    #include "encls.h"
    @@ -743,6 +746,51 @@ void sgx_update_lepubkeyhash(u64 *lepubkeyhash)
    wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0 + i, lepubkeyhash[i]);
    }

    +const struct file_operations sgx_provision_fops = {
    + .owner = THIS_MODULE,
    +};
    +
    +static struct miscdevice sgx_dev_provision = {
    + .minor = MISC_DYNAMIC_MINOR,
    + .name = "sgx_provision",
    + .nodename = "sgx_provision",
    + .fops = &sgx_provision_fops,
    +};
    +
    +/**
    + * sgx_set_attribute() - Update allowed attributes given file descriptor
    + * @allowed_attributes: Pointer to allowed enclave attributes
    + * @attribute_fd: File descriptor for specific attribute
    + *
    + * Append enclave attribute indicated by file descriptor to allowed
    + * attributes. Currently only SGX_ATTR_PROVISIONKEY indicated by
    + * /dev/sgx_provision is supported.
    + *
    + * Return:
    + * -0: SGX_ATTR_PROVISIONKEY is appended to allowed_attributes
    + * -EINVAL: Invalid, or not supported file descriptor
    + */
    +int sgx_set_attribute(unsigned long *allowed_attributes,
    + unsigned int attribute_fd)
    +{
    + struct file *file;
    +
    + file = fget(attribute_fd);
    + if (!file)
    + return -EINVAL;
    +
    + if (file->f_op != &sgx_provision_fops) {
    + fput(file);
    + return -EINVAL;
    + }
    +
    + *allowed_attributes |= SGX_ATTR_PROVISIONKEY;
    +
    + fput(file);
    + return 0;
    +}
    +EXPORT_SYMBOL_GPL(sgx_set_attribute);
    +
    static int __init sgx_init(void)
    {
    int ret;
    @@ -759,6 +807,10 @@ static int __init sgx_init(void)
    goto err_page_cache;
    }

    + ret = misc_register(&sgx_dev_provision);
    + if (ret)
    + goto err_kthread;
    +
    /*
    * Always try to initialize the native *and* KVM drivers.
    * The KVM driver is less picky than the native one and
    @@ -770,10 +822,13 @@ static int __init sgx_init(void)
    ret = sgx_drv_init();

    if (sgx_vepc_init() && ret)
    - goto err_kthread;
    + goto err_provision;

    return 0;

    +err_provision:
    + misc_deregister(&sgx_dev_provision);
    +
    err_kthread:
    kthread_stop(ksgxd_tsk);

    \
     
     \ /
      Last update: 2021-04-07 12:09    [W:2.663 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site