From patchwork Sun Mar 19 13:41:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 9632567 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D8BC9601E9 for ; Sun, 19 Mar 2017 13:44:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CA17E284CB for ; Sun, 19 Mar 2017 13:44:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BEBA3284CE; Sun, 19 Mar 2017 13:44:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RCVD_IN_SORBS_SPAM,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id DFC9A284CB for ; Sun, 19 Mar 2017 13:44:08 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cpb5P-0000Ef-4D; Sun, 19 Mar 2017 13:41:31 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cpb5N-0000Dq-9S for xen-devel@lists.xenproject.org; Sun, 19 Mar 2017 13:41:29 +0000 Received: from [85.158.139.211] by server-10.bemta-5.messagelabs.com id 56/80-02186-88A8EC85; Sun, 19 Mar 2017 13:41:28 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrKIsWRWlGSWpSXmKPExsVyMfTGYd32rnM RBtvnqlh83zKZyYHR4/CHKywBjFGsmXlJ+RUJrBnrVkxnK1ieUPH55h3mBsaJHl2MXBxCAjMY Jdo+v2MGcVgEPrBI3D01B8yREJjGKtGx7RBrFyMnkBMjcXDHB0YIu0pi3rvbLCC2kICSxJbJj xkhRu1iklh+5hAbSEJYQE9i8rfbYA1sAvoST9deA5rKAdTsJvHpGhdIWEQgUOLcvsdgy5gFeh gl7jRNZIXoDZA4cP4xO4jNIqAqsXjBObA5vAKuEncmdrNAHCEncfNcJzOIzQk0c+mnmVAHuUp cf/eOfQKj0AJGhlWMGsWpRWWpRbpGJnpJRZnpGSW5iZk5uoYGpnq5qcXFiempOYlJxXrJ+bmb GIHBWM/AwLiD8eZkv0OMkhxMSqK8KoInIoT4kvJTKjMSizPii0pzUosPMcpwcChJ8M5tBcoJF qWmp1akZeYA4wImLcHBoyTCexMkzVtckJhbnJkOkTrFaMnx4NSuN0wcc2bvBpKf+g+/YRJiyc vPS5US590C0iAA0pBRmgc3Dha7lxhlpYR5GRkYGIR4ClKLcjNLUOVfMYpzMCoJ88q1AU3hycw rgdv6CuggJqCDEn8eATmoJBEhJdXA6DOzSML12sUms9aJguEx8R8a987iP+1x0MSsc932pHfV 2+TXNZ02sv6TFr0nLmHrtuw5D2tzdxeq+kcWRvQnX/4b/+n778nh7Gvm8n7W4bZSzf5hb2hf2 2N/WXHPc/vgqTxM6QdaLebqNIcZ79pscL7n0MtNfvJWdgv5XLm3Mj7Y6p9f+Xa9EktxRqKhFn NRcSIAIBrYcNgCAAA= X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-12.tower-206.messagelabs.com!1489930886!53954863!1 X-Originating-IP: [209.85.216.195] X-SpamReason: No, hits=1.2 required=7.0 tests=HOT_NASTY X-StarScan-Received: X-StarScan-Version: 9.2.3; banners=-,-,- X-VirusChecked: Checked Received: (qmail 34752 invoked from network); 19 Mar 2017 13:41:27 -0000 Received: from mail-qt0-f195.google.com (HELO mail-qt0-f195.google.com) (209.85.216.195) by server-12.tower-206.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 19 Mar 2017 13:41:27 -0000 Received: by mail-qt0-f195.google.com with SMTP id r5so14549262qtb.2 for ; Sun, 19 Mar 2017 06:41:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=fQZ4cYxQi64TYhROSKDCloTeLaC86JOgeEi0lhdOY58=; b=hE/qYGS612sVUfXYD2Ue/nMu6Mn7GE/MJEJyZ9mCDT+aFpM8IrLv7VUbzVeTqfvOD3 pL84HiTWC58FeugxJTav1bkipx/6gwD6IMNxcLqNmQ2fQn/NStOQxjBuvGRpz0tL3aPa wb8b9Xca7sJIsBiR4QBAQ1YryQ8O+jkTBw4t51WJneKoaRrEEQgb8YUjAm4Jk7Y/fgeE MrSNYZz8fnhTaVpBA4x72FLUuzx8KSPgdn/meSG7w7zoid4TDkZgtBMVcQOzjGpxviot f+g4shxd3moHRu1InZAij2Q+nW/SYqjSJXt5EvFpO8uyZNm8AfwAFK5JvVHHp/zmDCx8 IDww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=fQZ4cYxQi64TYhROSKDCloTeLaC86JOgeEi0lhdOY58=; b=V7Q7en543qdoIvbBnLPBw2wZSScUNv95Pc0TWjsufqdf8y6Hog/4hFo3eA6sPRfjCT b0dCD5pgd8eF6jeoaeJmu3NAhm3jprCPUmMCXngWNcferTJf8r2U86XKGfiKMMyC6i6l folZ5jyNoZDj8vSUX9ibj3Hp5zo2nwXnuV1nnzgXekwtYy24sSDtFThgyuk4mGcAF+VU TOtKyknXe9s6miEM70JLhabJiPvW3TPWAeP7GOZjoh8IR2+D0kTTmK6mWR57X8TLWkKz Q+YDRVJypFgmRPQkzfJRIY5Am5y2RzH9rMete4r5p0QCtca2KlmFx5tnVE6qA5mRss31 79gQ== X-Gm-Message-State: AFeK/H2henB/nmNEeE7z8xJ6MYM07Iii9I5/9JsyQmCzjvqVjBQ5BmOf2t4u47PwdKmXhQ== X-Received: by 10.237.38.65 with SMTP id z59mr22319185qtc.215.1489930886228; Sun, 19 Mar 2017 06:41:26 -0700 (PDT) Received: from build-external.dumpdata.com (209-6-196-81.c3-0.smr-ubr2.sbo-smr.ma.cable.rcn.com. [209.6.196.81]) by smtp.gmail.com with ESMTPSA id q31sm10286548qta.22.2017.03.19.06.41.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 19 Mar 2017 06:41:25 -0700 (PDT) From: Konrad Rzeszutek Wilk X-Google-Original-From: Konrad Rzeszutek Wilk To: xen-devel@lists.xenproject.org, ian.jackson@citrix.com, wei.liu2@citrix.com Date: Sun, 19 Mar 2017 09:41:09 -0400 Message-Id: <1489930872-7823-3-git-send-email-konrad.wilk@oracle.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1489930872-7823-1-git-send-email-konrad.wilk@oracle.com> References: <1489930872-7823-1-git-send-email-konrad.wilk@oracle.com> Cc: andrew.cooper3@citrix.com, Ian Jackson , jbeulich@suse.com Subject: [Xen-devel] [PATCH v1 2/5] xen/libxc: Move TMEM_AUTH to XEN_SYSCTL_TMEM_OP_SET_AUTH X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP which surprisingly (or maybe not) looks like XEN_SYSCTL_TMEM_OP_SET_POOLS. This hypercall came about, as explained in docs/misc/tmem-internals.html: When tmem was first proposed to the linux kernel mailing list (LKML), there was concern expressed about security of shared ephemeral pools. The initial tmem implementation only required a client to provide a 128-bit UUID to identify a shared pool, and the linux-side tmem implementation obtained this UUID from the superblock of the shared filesystem (in ocfs2). It was pointed out on LKML that the UUID was essentially a security key and any malicious domain that guessed it would have access to any data from the shared filesystem that found its way into tmem. .. As a result, a Xen boot option -- tmem_shared_auth; -- was added. The option defaults to disabled, but when it is enabled, management tools must explicitly authenticate (or may explicitly deny) shared pool access to any client. On Xen, this is done with the xm tmem-shared-auth command. " However the implementation has some rather large holes: a) The hypercall was accessed from any guest. b) If the ->domain id value is 0xFFFF then one can toggle the tmem_global.shared_auth knob on/off. That with a) made it pretty bad. c) If one toggles the tmem_global.shared_auth off, then the 'tmem_shared_auth=1' bootup parameter is ignored and one can join any shared pool (if UUID is known)! d) If the 'tmem_shared_auth=1' and tmem_global.shared_auth is set to 1, then one can only join an shared pool if the UUID has been set by 'xl tmem-shared-auth'. Otherwise the joining of a pool fails and a non-shared pool is created (without errors to guest). Not exactly sure if the shared pool creation at that point should error out or not. e) If a guest is migrated, the policy values (which UUID can be shared, whether tmem_global.shared_auth is set, etc) are completely ignored. This patch only fixes a) and only allows the hypercall to be called by the control domain. Subsequent patches will fix the remaining issues. We also have to call client_create as the guest at this point may not have done any tmem hypercalls - and hence the '->tmem' from 'struct domain' is still NULL. Us calling client_create fixes this. Signed-off-by: Konrad Rzeszutek Wilk --- Cc: Ian Jackson Cc: Wei Liu --- tools/libxc/include/xenctrl.h | 2 +- tools/libxc/xc_tmem.c | 52 +++++++++++++----------------------------- xen/common/tmem.c | 7 +++--- xen/common/tmem_control.c | 48 ++++++++++++++++++++++++++++++++++++++ xen/include/public/sysctl.h | 4 +++- xen/include/public/tmem.h | 9 ++++---- xen/include/xen/tmem_control.h | 2 ++ xen/include/xen/tmem_xen.h | 1 - 8 files changed, 78 insertions(+), 47 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 2d97d36..e2b4cc1 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1901,7 +1901,7 @@ int xc_tmem_control_oid(xc_interface *xch, int32_t pool_id, uint32_t subop, int xc_tmem_control(xc_interface *xch, int32_t pool_id, uint32_t subop, uint32_t cli_id, uint32_t len, uint32_t arg, void *buf); -int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int arg1); +int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int enable); int xc_tmem_save(xc_interface *xch, int dom, int live, int fd, int field_marker); int xc_tmem_save_extra(xc_interface *xch, int dom, int fd, int field_marker); void xc_tmem_save_done(xc_interface *xch, int dom); diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c index 181de48..5f5e18f 100644 --- a/tools/libxc/xc_tmem.c +++ b/tools/libxc/xc_tmem.c @@ -22,30 +22,6 @@ #include #include -static int do_tmem_op(xc_interface *xch, tmem_op_t *op) -{ - int ret; - DECLARE_HYPERCALL_BOUNCE(op, sizeof(*op), XC_HYPERCALL_BUFFER_BOUNCE_BOTH); - - if ( xc_hypercall_bounce_pre(xch, op) ) - { - PERROR("Could not bounce buffer for tmem op hypercall"); - return -EFAULT; - } - - ret = xencall1(xch->xcall, __HYPERVISOR_tmem_op, - HYPERCALL_BUFFER_AS_ARG(op)); - if ( ret < 0 ) - { - if ( errno == EACCES ) - DPRINTF("tmem operation failed -- need to" - " rebuild the user-space tool set?\n"); - } - xc_hypercall_bounce_post(xch, op); - - return ret; -} - int xc_tmem_control(xc_interface *xch, int32_t pool_id, uint32_t cmd, @@ -69,7 +45,8 @@ int xc_tmem_control(xc_interface *xch, sysctl.u.tmem_op.oid.oid[1] = 0; sysctl.u.tmem_op.oid.oid[2] = 0; - if ( cmd == XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO ) + if ( cmd == XEN_SYSCTL_TMEM_OP_SET_CLIENT_INFO || + cmd == XEN_SYSCTL_TMEM_OP_SET_AUTH ) HYPERCALL_BOUNCE_SET_DIR(buf, XC_HYPERCALL_BUFFER_BOUNCE_IN); if ( len ) { @@ -176,22 +153,25 @@ static int xc_tmem_uuid_parse(char *uuid_str, uint64_t *uuid_lo, uint64_t *uuid_ int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, - int arg1) + int enable) { - tmem_op_t op; - - op.cmd = TMEM_AUTH; - op.pool_id = 0; - op.u.creat.arg1 = cli_id; - op.u.creat.flags = arg1; - if ( xc_tmem_uuid_parse(uuid_str, &op.u.creat.uuid[0], - &op.u.creat.uuid[1]) < 0 ) + xen_tmem_pool_info_t pool = { + .flags.u.auth = enable, + .id = 0, + .n_pages = 0, + .uuid[0] = 0, + .uuid[1] = 0, + }; + if ( xc_tmem_uuid_parse(uuid_str, &pool.uuid[0], + &pool.uuid[1]) < 0 ) { PERROR("Can't parse uuid, use xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"); return -1; } - - return do_tmem_op(xch, &op); + return xc_tmem_control(xch, 0 /* pool_id */, + XEN_SYSCTL_TMEM_OP_SET_AUTH, + cli_id, sizeof(pool), + 0 /* arg */, &pool); } /* Save/restore/live migrate */ diff --git a/xen/common/tmem.c b/xen/common/tmem.c index b92793d..504e9eb 100644 --- a/xen/common/tmem.c +++ b/xen/common/tmem.c @@ -1604,8 +1604,8 @@ fail: /************ TMEM CONTROL OPERATIONS ************************************/ -static int tmemc_shared_pool_auth(domid_t cli_id, uint64_t uuid_lo, - uint64_t uuid_hi, bool_t auth) +int tmemc_shared_pool_auth(domid_t cli_id, uint64_t uuid_lo, + uint64_t uuid_hi, bool auth) { struct client *client; int i, free = -1; @@ -1908,7 +1908,8 @@ long do_tmem_op(tmem_cli_op_t uops) /* Acquire write lock for all commands at first. */ write_lock(&tmem_rwlock); - if ( op.cmd == TMEM_CONTROL || op.cmd == TMEM_RESTORE_NEW ) + if ( op.cmd == TMEM_CONTROL || op.cmd == TMEM_RESTORE_NEW || + op.cmd == TMEM_AUTH ) { rc = -EOPNOTSUPP; } diff --git a/xen/common/tmem_control.c b/xen/common/tmem_control.c index fc09814..cae3a95 100644 --- a/xen/common/tmem_control.c +++ b/xen/common/tmem_control.c @@ -448,6 +448,51 @@ static int tmemc_set_pools(int cli_id, return rc ? : idx; } +static int tmemc_auth_pools(int cli_id, + XEN_GUEST_HANDLE(xen_tmem_pool_info_t) pools, + uint32_t len) +{ + unsigned int i, idx; + int rc = 0; + unsigned int nr = len / sizeof(xen_tmem_pool_info_t); + struct client *client = tmem_client_from_cli_id(cli_id); + + if ( len % sizeof(xen_tmem_pool_info_t) ) + return -EINVAL; + + if ( nr > MAX_POOLS_PER_DOMAIN ) + return -E2BIG; + + if ( !guest_handle_okay(pools, nr) ) + return -EINVAL; + + if ( !client ) + { + client = client_create(cli_id); + if ( !client ) + return -ENOMEM; + } + + for ( idx = 0, i = 0; i < nr; i++ ) + { + xen_tmem_pool_info_t pool; + + if ( __copy_from_guest_offset(&pool, pools, i, 1 ) ) + return -EFAULT; + + rc = tmemc_shared_pool_auth(cli_id, pool.uuid[0], pool.uuid[1], + pool.flags.u.auth); + + if ( rc < 0 ) + break; + + idx++; + } + + /* And how many we have processed. */ + return rc ? : idx; +} + int tmem_control(struct xen_sysctl_tmem_op *op) { int ret; @@ -487,6 +532,9 @@ int tmem_control(struct xen_sysctl_tmem_op *op) case XEN_SYSCTL_TMEM_OP_SET_POOLS: /* TMEM_RESTORE_NEW */ ret = tmemc_set_pools(op->cli_id, op->u.pool, op->len); break; + case XEN_SYSCTL_TMEM_OP_SET_AUTH: /* TMEM_AUTH */ + ret = tmemc_auth_pools(op->cli_id, op->u.pool, op->len); + break; default: ret = do_tmem_control(op); break; diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 74a7462..74c24cc 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -772,6 +772,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t); #define XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB 8 #define XEN_SYSCTL_TMEM_OP_SET_POOLS 9 #define XEN_SYSCTL_TMEM_OP_SAVE_BEGIN 10 +#define XEN_SYSCTL_TMEM_OP_SET_AUTH 11 #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE 19 #define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV 20 #define XEN_SYSCTL_TMEM_OP_SAVE_END 21 @@ -824,7 +825,8 @@ struct xen_tmem_pool_info { struct { uint32_t persist:1, /* See TMEM_POOL_PERSIST. */ shared:1, /* See TMEM_POOL_SHARED. */ - rsv:2, + auth:1, /* See TMEM_POOL_AUTH. */ + rsv1:1, pagebits:8, /* TMEM_POOL_PAGESIZE_[SHIFT,MASK]. */ rsv2:12, version:8; /* TMEM_POOL_VERSION_[SHIFT,MASK]. */ diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h index b9f3537..aa0aafa 100644 --- a/xen/include/public/tmem.h +++ b/xen/include/public/tmem.h @@ -51,10 +51,9 @@ #define TMEM_XCHG 10 #endif -/* Privileged commands to HYPERVISOR_tmem_op() */ -#define TMEM_AUTH 101 -#define TMEM_RESTORE_NEW 102 /* Now called via XEN_SYSCTL_tmem_op as - XEN_SYSCTL_TMEM_OP_SET_POOL. */ +/* Privileged commands now called via XEN_SYSCTL_tmem_op. */ +#define TMEM_AUTH 101 /* as XEN_SYSCTL_TMEM_OP_SET_AUTH. */ +#define TMEM_RESTORE_NEW 102 /* as XEN_SYSCTL_TMEM_OP_SET_POOL. */ /* Bits for HYPERVISOR_tmem_op(TMEM_NEW_POOL) */ #define TMEM_POOL_PERSIST 1 @@ -93,7 +92,7 @@ struct tmem_op { uint64_t uuid[2]; uint32_t flags; uint32_t arg1; - } creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH */ + } creat; /* for cmd == TMEM_NEW_POOL. */ struct { #if __XEN_INTERFACE_VERSION__ < 0x00040600 uint64_t oid[3]; diff --git a/xen/include/xen/tmem_control.h b/xen/include/xen/tmem_control.h index 91c185e..ad04cf7 100644 --- a/xen/include/xen/tmem_control.h +++ b/xen/include/xen/tmem_control.h @@ -22,6 +22,8 @@ struct client *client_create(domid_t cli_id); int do_tmem_new_pool(domid_t this_cli_id, uint32_t d_poolid, uint32_t flags, uint64_t uuid_lo, uint64_t uuid_hi); +int tmemc_shared_pool_auth(domid_t cli_id, uint64_t uuid_lo, + uint64_t uuid_hi, bool auth); #endif /* CONFIG_TMEM */ #endif /* __XEN_TMEM_CONTROL_H__ */ diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h index b6bd61b..70cc108 100644 --- a/xen/include/xen/tmem_xen.h +++ b/xen/include/xen/tmem_xen.h @@ -198,7 +198,6 @@ static inline int tmem_get_tmemop_from_client(tmem_op_t *op, tmem_cli_op_t uops) switch ( cop.cmd ) { case TMEM_NEW_POOL: u = XLAT_tmem_op_u_creat; break; - case TMEM_AUTH: u = XLAT_tmem_op_u_creat; break; default: u = XLAT_tmem_op_u_gen ; break; } XLAT_tmem_op(op, &cop);