|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 11/20] xen/domctl: Merge set_gnttab_limits into createdomain
XEN_DOMCTL_set_gnttab_limits is a fairly new hypercall, and is strictly
mandatory. Adding support for it introduced a state where a domain has a
mostly un-constructed grant table, and there were cases where mis-ordering of
toolstack hypercalls could cause a NULL pointer deference in the hypervisor.
In fixing this, the grant table initialisation code became very tangled.
As the settings are mandatory, delete XEN_DOMCTL_set_gnttab_limits (including
XSM hooks and libxc wrappers) and retain the functionality in
XEN_DOMCTL_createdomain.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
CC: David Scott <dave@xxxxxxxxxx>
CC: Jon Ludlam <jonathan.ludlam@xxxxxxxxxxxxx>
CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
CC: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Hypervisor side cleanup is present in later patchs
---
tools/flask/policy/modules/dom0.te | 2 +-
tools/flask/policy/modules/xen.if | 2 +-
tools/helpers/init-xenstore-domain.c | 19 ++++++++-----------
tools/libxc/include/xenctrl.h | 13 -------------
tools/libxc/xc_domain.c | 13 -------------
tools/libxl/libxl_create.c | 2 ++
tools/libxl/libxl_dom.c | 6 ------
tools/ocaml/libs/xc/xenctrl.ml | 2 ++
tools/ocaml/libs/xc/xenctrl.mli | 2 ++
tools/ocaml/libs/xc/xenctrl_stubs.c | 8 +++++++-
tools/python/xen/lowlevel/xc/xc.c | 2 ++
xen/common/domctl.c | 34 ++++++++++++++++++++++++++--------
xen/include/public/domctl.h | 10 +++-------
xen/xsm/flask/hooks.c | 3 ---
xen/xsm/flask/policy/access_vectors | 2 --
15 files changed, 54 insertions(+), 66 deletions(-)
diff --git a/tools/flask/policy/modules/dom0.te
b/tools/flask/policy/modules/dom0.te
index 4eb3843..dfdcdcd 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain {
};
allow dom0_t dom0_t:domain2 {
set_cpuid gettsc settsc setscheduler set_vnumainfo
- get_vnumainfo psr_cmt_op psr_alloc set_gnttab_limits
+ get_vnumainfo psr_cmt_op psr_alloc
};
allow dom0_t dom0_t:resource { add remove };
diff --git a/tools/flask/policy/modules/xen.if
b/tools/flask/policy/modules/xen.if
index 7dc25be..5af984c 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -52,7 +52,7 @@ define(`create_domain_common', `
settime setdomainhandle getvcpucontext set_misc_info };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_vnumainfo get_vnumainfo cacheflush
- psr_cmt_op psr_alloc soft_reset set_gnttab_limits };
+ psr_cmt_op psr_alloc soft_reset };
allow $1 $2:security check_context;
allow $1 $2:shadow enable;
allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage
mmuext_op updatemp };
diff --git a/tools/helpers/init-xenstore-domain.c
b/tools/helpers/init-xenstore-domain.c
index 89c329c..4771750 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -67,6 +67,14 @@ static int build(xc_interface *xch)
.ssidref = SECINITSID_DOMU,
.flags = XEN_DOMCTL_CDF_xs_domain,
.max_evtchn_port = -1, /* No limit. */
+
+ /*
+ * 1 grant frame is enough: we don't need many grants.
+ * Mini-OS doesn't like less than 4, though, so use 4.
+ * 128 maptrack frames: 256 entries per frame, enough for 32768
domains.
+ */
+ .max_grant_frames = 4,
+ .max_maptrack_frames = 128,
};
xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
@@ -104,17 +112,6 @@ static int build(xc_interface *xch)
fprintf(stderr, "xc_domain_setmaxmem failed\n");
goto err;
}
- /*
- * 1 grant frame is enough: we don't need many grants.
- * Mini-OS doesn't like less than 4, though, so use 4.
- * 128 maptrack frames: 256 entries per frame, enough for 32768 domains.
- */
- rv = xc_domain_set_gnttab_limits(xch, domid, 4, 128);
- if ( rv )
- {
- fprintf(stderr, "xc_domain_set_gnttab_limits failed\n");
- goto err;
- }
rv = xc_domain_set_memmap_limit(xch, domid, limit_kb);
if ( rv )
{
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 88a175f..0c7c07c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1071,19 +1071,6 @@ int xc_domain_set_access_required(xc_interface *xch,
*/
int xc_domain_set_virq_handler(xc_interface *xch, uint32_t domid, int virq);
-/**
- * Set the maximum number of grant frames and maptrack frames a domain
- * can have. Must be used at domain setup time and only then.
- *
- * @param xch a handle to an open hypervisor interface
- * @param domid the domain id
- * @param grant_frames max. number of grant frames
- * @param maptrack_frames max. number of maptrack frames
- */
-int xc_domain_set_gnttab_limits(xc_interface *xch, uint32_t domid,
- uint32_t grant_frames,
- uint32_t maptrack_frames);
-
/*
* CPUPOOL MANAGEMENT FUNCTIONS
*/
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 2bc695c..e8d0734 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2256,19 +2256,6 @@ int xc_domain_set_virq_handler(xc_interface *xch,
uint32_t domid, int virq)
return do_domctl(xch, &domctl);
}
-int xc_domain_set_gnttab_limits(xc_interface *xch, uint32_t domid,
- uint32_t grant_frames,
- uint32_t maptrack_frames)
-{
- DECLARE_DOMCTL;
-
- domctl.cmd = XEN_DOMCTL_set_gnttab_limits;
- domctl.domain = domid;
- domctl.u.set_gnttab_limits.grant_frames = grant_frames;
- domctl.u.set_gnttab_limits.maptrack_frames = maptrack_frames;
- return do_domctl(xch, &domctl);
-}
-
/* Plumbing Xen with vNUMA topology */
int xc_domain_setvnuma(xc_interface *xch,
uint32_t domid,
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2cb3460..c8eb59c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -566,6 +566,8 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
struct xen_domctl_createdomain create = {
.ssidref = info->ssidref,
.max_evtchn_port = b_info->event_channels,
+ .max_grant_frames = b_info->max_grant_frames,
+ .max_maptrack_frames = b_info->max_maptrack_frames,
};
if (info->type != LIBXL_DOMAIN_TYPE_PV) {
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 227e6cf..9194109 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -358,12 +358,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
return ERROR_FAIL;
}
- if (xc_domain_set_gnttab_limits(ctx->xch, domid, info->max_grant_frames,
- info->max_maptrack_frames) != 0) {
- LOG(ERROR, "Couldn't set grant table limits");
- return ERROR_FAIL;
- }
-
/*
* Check if the domain has any CPU or node affinity already. If not, try
* to build up the latter via automatic NUMA placement. In fact, in case
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 6dc0dd7..7c8d6ab 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -64,6 +64,8 @@ type domctl_create_config =
handle: string;
flags: domain_create_flag list;
max_evtchn_port: int32;
+ max_grant_frames: int32;
+ max_maptrack_frames: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index b0fec24..f150a5d 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -56,6 +56,8 @@ type domctl_create_config = {
handle: string;
flags: domain_create_flag list;
max_evtchn_port: int32;
+ max_grant_frames: int32;
+ max_maptrack_frames: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index c022de9..882828f 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -128,7 +128,9 @@ CAMLprim value stub_xc_domain_create(value xch, value
config)
#define VAL_HANDLE Field(config, 1)
#define VAL_FLAGS Field(config, 2)
#define VAL_MAX_EVTCHN_PORT Field(config, 3)
-#define VAL_ARCH Field(config, 4)
+#define VAL_MAX_GRANT_FRAMES Field(config, 4)
+#define VAL_MAX_MAPTRACK_FRAMES Field(config, 5)
+#define VAL_ARCH Field(config, 6)
uint32_t domid = 0;
int result;
@@ -136,6 +138,8 @@ CAMLprim value stub_xc_domain_create(value xch, value
config)
struct xen_domctl_createdomain cfg = {
.ssidref = Int32_val(VAL_SSIDREF),
.max_evtchn_port = Int32_val(VAL_MAX_EVTCHN_PORT),
+ .max_grant_frames = Int32_val(VAL_MAX_GRANT_FRAMES),
+ .max_maptrack_frames = Int32_val(VAL_MAX_MAPTRACK_FRAMES),
};
domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE));
@@ -164,6 +168,8 @@ CAMLprim value stub_xc_domain_create(value xch, value
config)
}
#undef VAL_ARCH
+#undef VAL_MAX_MAPTRACK_FRAMES
+#undef VAL_MAX_GRANT_FRAMES
#undef VAL_MAX_EVTCHN_PORT
#undef VAL_FLAGS
#undef VAL_HANDLE
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index a307de7..db99a52 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -126,6 +126,8 @@ static PyObject *pyxc_domain_create(XcObject *self,
0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
},
.max_evtchn_port = -1, /* No limit. */
+ .max_grant_frames = 32,
+ .max_maptrack_frames = 1024,
};
static char *kwd_list[] = { "domid", "ssidref", "handle", "flags",
"target", NULL };
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 14dab56..d8ba461 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -539,14 +539,37 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
break;
}
+ /* Stash the new domid for the toolstack. */
+ op->domain = d->domain_id;
+ copyback = true;
+
d->max_evtchn_port =
min_t(unsigned int, op->u.createdomain.max_evtchn_port, INT_MAX);
- ret = 0;
- op->domain = d->domain_id;
- copyback = 1;
+ ret = grant_table_set_limits(d, op->u.createdomain.max_grant_frames,
+ op->u.createdomain.max_maptrack_frames);
+ if ( !ret )
+ goto createdomain_fail_late;
+
d = NULL;
break;
+
+ createdomain_fail_late:
+ /*
+ * We've hit an error after putting the domain into the domain list,
+ * meaning that other entities in the system can refer to it.
+ *
+ * Unwinding is substantially more complicated, and without
+ * returning success, the toolstack wont know to clean up.
+ *
+ * Reuse the continuation logic to turn this hypercall into a
+ * destroydomain on behalf of the toolstack.
+ */
+ op->cmd = XEN_DOMCTL_destroydomain;
+ d = NULL;
+
+ ret = hypercall_create_continuation(__HYPERVISOR_domctl, "h",
u_domctl);
+ break;
}
case XEN_DOMCTL_max_vcpus:
@@ -1114,11 +1137,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
copyback = 1;
break;
- case XEN_DOMCTL_set_gnttab_limits:
- ret = grant_table_set_limits(d, op->u.set_gnttab_limits.grant_frames,
- op->u.set_gnttab_limits.maptrack_frames);
- break;
-
default:
ret = arch_do_domctl(op, d, u_domctl);
break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 515671e..424f0a8 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -71,6 +71,8 @@ struct xen_domctl_createdomain {
* mapping space, xenheap, etc) a guest may consume.
*/
uint32_t max_evtchn_port;
+ uint32_t max_grant_frames;
+ uint32_t max_maptrack_frames;
struct xen_arch_domainconfig arch;
};
@@ -1065,11 +1067,6 @@ struct xen_domctl_psr_alloc {
uint64_t data; /* IN/OUT */
};
-struct xen_domctl_set_gnttab_limits {
- uint32_t grant_frames; /* IN */
- uint32_t maptrack_frames; /* IN */
-};
-
/* XEN_DOMCTL_vuart_op */
struct xen_domctl_vuart_op {
#define XEN_DOMCTL_VUART_OP_INIT 0
@@ -1168,7 +1165,7 @@ struct xen_domctl {
#define XEN_DOMCTL_monitor_op 77
#define XEN_DOMCTL_psr_alloc 78
#define XEN_DOMCTL_soft_reset 79
-#define XEN_DOMCTL_set_gnttab_limits 80
+/* #define XEN_DOMCTL_set_gnttab_limits 80 - Moved into
XEN_DOMCTL_createdomain */
#define XEN_DOMCTL_vuart_op 81
#define XEN_DOMCTL_gdbsx_guestmemio 1000
#define XEN_DOMCTL_gdbsx_pausevcpu 1001
@@ -1229,7 +1226,6 @@ struct xen_domctl {
struct xen_domctl_psr_cmt_op psr_cmt_op;
struct xen_domctl_monitor_op monitor_op;
struct xen_domctl_psr_alloc psr_alloc;
- struct xen_domctl_set_gnttab_limits set_gnttab_limits;
struct xen_domctl_vuart_op vuart_op;
uint8_t pad[128];
} u;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 1978703..cccd1c7 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -740,9 +740,6 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_soft_reset:
return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
- case XEN_DOMCTL_set_gnttab_limits:
- return current_has_perm(d, SECCLASS_DOMAIN2,
DOMAIN2__SET_GNTTAB_LIMITS);
-
default:
return avc_unknown_permission("domctl", cmd);
}
diff --git a/xen/xsm/flask/policy/access_vectors
b/xen/xsm/flask/policy/access_vectors
index deecfa2..6f6e969 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -246,8 +246,6 @@ class domain2
mem_sharing
# XEN_DOMCTL_psr_alloc
psr_alloc
-# XEN_DOMCTL_set_gnttab_limits
- set_gnttab_limits
}
# Similar to class domain, but primarily contains domctls related to HVM
domains
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |