|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3 01/11] libxl: Move dm user determination logic into a helper function
To reliably kill an untrusted devicemodel, we need to know not only
its pid, but its uid. In preparation for this, move the userid
determination logic into a helper function.
Create a new field, `dm_runas`, in libxl__domain_build_state to store
the value during domain creation.
This change also removes unnecessary duplication of the argument
construction code.
While here, clean up some minor CODING_STYLE infractions (space
between * and variable name).
No functional change intended.
While here, delete some trailing whitespace.
Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
v2:
- Remove unnecessary space between * and dm_runas
- Additional code clean-up
- Delete trailing whitespace
CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Anthony Perard <anthony.perard@xxxxxxxxxx>
---
tools/libxl/libxl_dm.c | 260 +++++++++++++++++++----------------
tools/libxl/libxl_internal.h | 22 +--
2 files changed, 151 insertions(+), 131 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5698fe8af3..bbcbc94b6c 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -65,6 +65,131 @@ static int libxl__create_qemu_logfile(libxl__gc *gc, char
*name)
return logfile_w;
}
+/*
+ * userlookup_helper_getpwnam(libxl__gc*, const char *user,
+ * struct passwd **pwd_r);
+ *
+ * userlookup_helper_getpwuid(libxl__gc*, uid_t uid,
+ * struct passwd **pwd_r);
+ *
+ * returns 1 if the user was found, 0 if it was not, -1 on error
+ */
+#define DEFINE_USERLOOKUP_HELPER(NAME,SPEC_TYPE,STRUCTNAME,SYSCONF) \
+ static int userlookup_helper_##NAME(libxl__gc *gc, \
+ SPEC_TYPE spec, \
+ struct STRUCTNAME *resultbuf, \
+ struct STRUCTNAME **out) \
+ { \
+ struct STRUCTNAME *resultp = NULL; \
+ char *buf = NULL; \
+ long buf_size; \
+ int ret; \
+ \
+ buf_size = sysconf(SYSCONF); \
+ if (buf_size < 0) { \
+ buf_size = 2048; \
+ LOG(DEBUG, \
+ "sysconf failed, setting the initial buffer size to %ld", \
+ buf_size); \
+ } \
+ \
+ while (1) { \
+ buf = libxl__realloc(gc, buf, buf_size); \
+ ret = NAME##_r(spec, resultbuf, buf, buf_size, &resultp); \
+ if (ret == ERANGE) { \
+ buf_size += 128; \
+ continue; \
+ } \
+ if (ret != 0) \
+ return ERROR_FAIL; \
+ if (resultp != NULL) { \
+ if (out) *out = resultp; \
+ return 1; \
+ } \
+ return 0; \
+ } \
+ }
+
+DEFINE_USERLOOKUP_HELPER(getpwnam, const char*, passwd, _SC_GETPW_R_SIZE_MAX);
+DEFINE_USERLOOKUP_HELPER(getpwuid, uid_t, passwd, _SC_GETPW_R_SIZE_MAX);
+
+static int libxl__domain_get_device_model_uid(libxl__gc *gc,
+ libxl__dm_spawn_state *dmss)
+{
+ int guest_domid = dmss->guest_domid;
+ libxl__domain_build_state *const state = dmss->build_state;
+ const libxl_domain_build_info *b_info = &dmss->guest_config->b_info;
+
+ struct passwd *user_base, user_pwbuf;
+ int ret;
+ char *user;
+
+ /* Only qemu-upstream can run as a different uid */
+ if (b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN)
+ return 0;
+
+ user = b_info->device_model_user;
+ if (user)
+ goto end_search;
+
+ if (!libxl_defbool_val(b_info->dm_restrict)) {
+ LOGD(DEBUG, guest_domid,
+ "dm_restrict disabled, starting QEMU as root");
+ return 0;
+ }
+
+ user = GCSPRINTF("%s%d", LIBXL_QEMU_USER_BASE, guest_domid);
+ ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, 0);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ goto end_search;
+
+ ret = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_RANGE_BASE,
+ &user_pwbuf, &user_base);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ struct passwd *user_clash, user_clash_pwbuf;
+ uid_t intended_uid = user_base->pw_uid + guest_domid;
+ ret = userlookup_helper_getpwuid(gc, intended_uid,
+ &user_clash_pwbuf, &user_clash);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ LOGD(ERROR, guest_domid,
+ "wanted to use uid %ld (%s + %d) but that is user %s !",
+ (long)intended_uid, LIBXL_QEMU_USER_RANGE_BASE,
+ guest_domid, user_clash->pw_name);
+ return ERROR_FAIL;
+ }
+ LOGD(DEBUG, guest_domid, "using uid %ld", (long)intended_uid);
+ user = GCSPRINTF("%ld:%ld", (long)intended_uid,
+ (long)user_base->pw_gid);
+ goto end_search;
+ }
+
+ user = LIBXL_QEMU_USER_SHARED;
+ ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, 0);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ LOGD(WARN, guest_domid, "Could not find user %s%d, falling back to %s",
+ LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED);
+ goto end_search;
+ }
+
+ LOGD(ERROR, guest_domid,
+ "Could not find user %s%d or %s or range base pseudo-user %s, cannot
restrict",
+ LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED,
+ LIBXL_QEMU_USER_RANGE_BASE);
+ return ERROR_INVAL;
+
+end_search:
+ state->dm_runas = user;
+ return 0;
+}
+
const char *libxl__domain_device_model(libxl__gc *gc,
const libxl_domain_build_info *info)
{
@@ -737,54 +862,6 @@ libxl__detect_gfx_passthru_kind(libxl__gc *gc,
return LIBXL_GFX_PASSTHRU_KIND_DEFAULT;
}
-/*
- * userlookup_helper_getpwnam(libxl__gc*, const char *user,
- * struct passwd **pwd_r);
- *
- * userlookup_helper_getpwuid(libxl__gc*, uid_t uid,
- * struct passwd **pwd_r);
- *
- * returns 1 if the user was found, 0 if it was not, -1 on error
- */
-#define DEFINE_USERLOOKUP_HELPER(NAME,SPEC_TYPE,STRUCTNAME,SYSCONF) \
- static int userlookup_helper_##NAME(libxl__gc *gc, \
- SPEC_TYPE spec, \
- struct STRUCTNAME *resultbuf, \
- struct STRUCTNAME **out) \
- { \
- struct STRUCTNAME *resultp = NULL; \
- char *buf = NULL; \
- long buf_size; \
- int ret; \
- \
- buf_size = sysconf(SYSCONF); \
- if (buf_size < 0) { \
- buf_size = 2048; \
- LOG(DEBUG, \
- "sysconf failed, setting the initial buffer size to %ld", \
- buf_size); \
- } \
- \
- while (1) { \
- buf = libxl__realloc(gc, buf, buf_size); \
- ret = NAME##_r(spec, resultbuf, buf, buf_size, &resultp); \
- if (ret == ERANGE) { \
- buf_size += 128; \
- continue; \
- } \
- if (ret != 0) \
- return ERROR_FAIL; \
- if (resultp != NULL) { \
- if (out) *out = resultp; \
- return 1; \
- } \
- return 0; \
- } \
- }
-
-DEFINE_USERLOOKUP_HELPER(getpwnam, const char*, passwd, _SC_GETPW_R_SIZE_MAX);
-DEFINE_USERLOOKUP_HELPER(getpwuid, uid_t, passwd, _SC_GETPW_R_SIZE_MAX);
-
/* colo mode */
enum {
LIBXL__COLO_NONE = 0,
@@ -928,11 +1005,9 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
const char *keymap = dm_keymap(guest_config);
char *machinearg;
flexarray_t *dm_args, *dm_envs;
- int i, connection, devid, ret;
+ int i, connection, devid;
uint64_t ram_size;
const char *path, *chardev;
- char *user = NULL;
- struct passwd *user_base, user_pwbuf;
dm_args = flexarray_make(gc, 16, 1);
dm_envs = flexarray_make(gc, 16, 1);
@@ -1414,10 +1489,10 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
char *chroot_dir = GCSPRINTF("%s/qemu-root-%d",
libxl__run_dir_path(), guest_domid);
int r;
-
+
flexarray_append(dm_args, "-xen-domid-restrict");
- /*
+ /*
* Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-<domid>
*
* There is no library function to do the equivalent of `rm
@@ -1425,7 +1500,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
* able to write any files, as the chroot would be owned by
* root, but it would be running as an unprivileged process.
* So in theory, old chroots should always be empty.
- *
+ *
* rmdir the directory before attempting to create
* it; if it returns anything other than ENOENT, fail domain
* creation.
@@ -1436,7 +1511,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
"failed to remove existing chroot dir %s", chroot_dir);
return ERROR_FAIL;
}
-
+
for (;;) {
r = mkdir(chroot_dir, 0000);
if (!r)
@@ -1538,7 +1613,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
continue;
}
- /*
+ /*
* If qemu isn't doing the interpreting, the parameter is
* always raw
*/
@@ -1563,7 +1638,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
continue;
}
- /*
+ /*
* We can't call libxl__blktap_devpath from
* libxl__device_disk_find_local_path for now because
* the bootloader is called before the disks are set
@@ -1685,71 +1760,9 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
break;
}
- if (b_info->device_model_user) {
- user = b_info->device_model_user;
- goto end_search;
- }
-
- if (!libxl_defbool_val(b_info->dm_restrict)) {
- LOGD(DEBUG, guest_domid,
- "dm_restrict disabled, starting QEMU as root");
- goto end_search;
- }
-
- user = GCSPRINTF("%s%d", LIBXL_QEMU_USER_BASE, guest_domid);
- ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, 0);
- if (ret < 0)
- return ret;
- if (ret > 0)
- goto end_search;
-
- ret = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_RANGE_BASE,
- &user_pwbuf, &user_base);
- if (ret < 0)
- return ret;
- if (ret > 0) {
- struct passwd *user_clash, user_clash_pwbuf;
- uid_t intended_uid = user_base->pw_uid + guest_domid;
- ret = userlookup_helper_getpwuid(gc, intended_uid,
- &user_clash_pwbuf, &user_clash);
- if (ret < 0)
- return ret;
- if (ret > 0) {
- LOGD(ERROR, guest_domid,
- "wanted to use uid %ld (%s + %d) but that is user %s !",
- (long)intended_uid, LIBXL_QEMU_USER_RANGE_BASE,
- guest_domid, user_clash->pw_name);
- return ERROR_FAIL;
- }
- LOGD(DEBUG, guest_domid, "using uid %ld", (long)intended_uid);
+ if (state->dm_runas && strcmp(state->dm_runas, "root")) {
flexarray_append(dm_args, "-runas");
- flexarray_append(dm_args,
- GCSPRINTF("%ld:%ld", (long)intended_uid,
- (long)user_base->pw_gid));
- user = NULL; /* we have taken care of it */
- goto end_search;
- }
-
- user = LIBXL_QEMU_USER_SHARED;
- ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, 0);
- if (ret < 0)
- return ret;
- if (ret > 0) {
- LOGD(WARN, guest_domid, "Could not find user %s%d, falling back to
%s",
- LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED);
- goto end_search;
- }
-
- LOGD(ERROR, guest_domid,
- "Could not find user %s%d or %s or range base pseudo-user %s, cannot
restrict",
- LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED,
- LIBXL_QEMU_USER_RANGE_BASE);
- return ERROR_INVAL;
-
-end_search:
- if (user != NULL && strcmp(user, "root")) {
- flexarray_append(dm_args, "-runas");
- flexarray_append(dm_args, user);
+ flexarray_append(dm_args, state->dm_runas);
}
}
flexarray_append(dm_args, NULL);
@@ -2303,6 +2316,11 @@ void libxl__spawn_local_dm(libxl__egc *egc,
libxl__dm_spawn_state *dmss)
rc = ERROR_FAIL;
goto out;
}
+
+ rc = libxl__domain_get_device_model_uid(gc, dmss);
+ if (rc)
+ goto out;
+
rc = libxl__build_device_model_args(gc, dm, domid, guest_config,
&args, &envs, state,
&dm_state_fd);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e498435e16..c4a43bd0b7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -332,7 +332,7 @@ struct libxl__ev_evtchn {
typedef struct libxl__ev_watch_slot {
LIBXL_SLIST_ENTRY(struct libxl__ev_watch_slot) empty;
} libxl__ev_watch_slot;
-
+
_hidden libxl__ev_xswatch *libxl__watch_slot_contents(libxl__gc *gc,
int slotnum);
@@ -484,7 +484,7 @@ struct libxl__ctx {
death_list /* sorted by domid */,
death_reported;
libxl__ev_xswatch death_watch;
-
+
LIBXL_LIST_HEAD(, libxl_evgen_disk_eject) disk_eject_evgens;
const libxl_childproc_hooks *childproc_hooks;
@@ -1131,9 +1131,11 @@ typedef struct {
libxl__file_reference pv_kernel;
libxl__file_reference pv_ramdisk;
- const char * shim_path;
- const char * shim_cmdline;
- const char * pv_cmdline;
+ const char *shim_path;
+ const char *shim_cmdline;
+ const char *pv_cmdline;
+
+ char *dm_runas;
xen_vmemrange_t *vmemranges;
uint32_t num_vmemranges;
@@ -1471,7 +1473,7 @@ _hidden void libxl__spawn_init(libxl__spawn_state*);
*
* what: string describing the spawned process, used for logging
*
- * Logs errors. A copy of "what" is taken.
+ * Logs errors. A copy of "what" is taken.
* Return values:
* < 0 error, *spawn is now Idle and need not be detached
* +1 caller is the parent, *spawn is Attached and must be detached
@@ -2750,10 +2752,10 @@ static inline void
libxl__device_disk_local_init(libxl__disk_local_state *dls)
dls->rc = 0;
}
-/*
+/*
* See if we can find a way to access a disk locally
*/
-_hidden char * libxl__device_disk_find_local_path(libxl__gc *gc,
+_hidden char * libxl__device_disk_find_local_path(libxl__gc *gc,
libxl_domid guest_domid,
const libxl_device_disk
*disk,
bool qdisk_direct);
@@ -3774,7 +3776,7 @@ struct libxl__dm_spawn_state {
_hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
-/*
+/*
* Called after forking but before executing the local devicemodel.
*/
_hidden int libxl__local_dm_preexec_restrict(libxl__gc *gc);
@@ -3963,7 +3965,7 @@ _hidden void libxl__remus_restore_setup(libxl__egc *egc,
*/
#define GCNEW_ARRAY(var, nmemb) \
((var) = libxl__calloc((gc), (nmemb), sizeof(*(var))))
-
+
/*
* Expression statement <type> *GCREALLOC_ARRAY(<type> *var, size_t nmemb);
* Uses libxl__gc *gc;
--
2.19.2
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |