Download raw body.
remove TOCTOU in vmd config and defer disk detection
vmd's config parsing opens disk images at parse time to attempt
detection of disk type (raw vs. qcow2). This is a TOCTOU as vmd doesn't
keep these files open and locked, so nothing prevents these files from
changing before vm launch.
For instance, if a vm owner has write-access to the disk image, nothing
prevents them from converting the image from raw to qcow2 or the
reverse. Nothing also prevents the file from being deleted. Nothing
prevents the file permissions from changing. Etc. etc.
realpath(3) is still used by parse.y so if the file *is* deleted, config
validation will still fail.
It would be excessive to keep the files open while vmd is running, so
instead defer to detection to open time. This shifts to using an enum
instead of ints with #DEFINEs to represent the 4 states:
invalid/uninitialized, detection needed (auto), raw, and qcow2.
I'm slowly chipping away at some fundamental issues in qcow2
support. After this, I have a new vmd process to delegate file opening
as the vm owner but may wait for after release for that type of change.
ok?
diff refs/heads/master refs/heads/vmd-parse_disk
commit - 6d57bcee831e743a742d07d09e624416a482c89a
commit + f69a11f3a0772e7b500e5ada30cb9bc522f060a8
blob - 0f3958c20c8b1b9bf704e8dca8caa441fb7284ea
blob + 9461759b25a9a96b1798f0d6d27a30510bc0782d
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -32,6 +32,7 @@
#include <imsg.h>
#include "proc.h"
+#include "virtio.h"
#include "vmd.h"
/* Supported bridge types */
@@ -190,10 +191,12 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui
int diskfds[VM_MAX_DISKS_PER_VM][VM_MAX_BASE_PER_DISK];
struct vmd_if *vif;
struct vmop_create_params *vmc = &vm->vm_params;
+ enum vm_disk_fmt type;
unsigned int i, j;
int fd = -1, cdromfd = -1, kernfd = -1;
int *tapfds = NULL;
- int n = 0, aflags, oflags, ret = -1;
+ int aflags, oflags, ret = -1;
+ ssize_t n = 0;
char ifname[IF_NAMESIZE], *s;
char path[PATH_MAX], base[PATH_MAX];
unsigned int unit;
@@ -330,7 +333,6 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui
oflags = O_RDWR | O_EXLOCK | O_NONBLOCK;
aflags = R_OK | W_OK;
for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
- /* Stat disk[i] to ensure it is a regular file */
if ((diskfds[i][j] = open(path, oflags)) == -1) {
log_warn("can't open disk %s",
vmc->vmc_disks[i]);
@@ -338,6 +340,10 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui
goto fail;
}
+ /*
+ * Check if it's a regular file and accessible to
+ * the user starting the vm.
+ */
if (vm_checkaccess(diskfds[i][j],
vmc->vmc_checkaccess & VMOP_CREATE_DISK,
uid, aflags) == -1) {
@@ -347,6 +353,23 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui
goto fail;
}
+ /* Identify the disk type if unknown. */
+ type = vmc->vmc_disktypes[i];
+ if (type == VMDF_AUTO) {
+ type = virtio_get_disktype(diskfds[i][j]);
+ vmc->vmc_disktypes[i] = type;
+ }
+ if (type != VMDF_RAW && type != VMDF_QCOW2) {
+ log_warnx("vm \"%s\" invalid disk format",
+ vmc->vmc_name);
+ ret = EINVAL;
+ goto fail;
+ }
+
+ /* We're done if it's not a QCOW disk image. */
+ if (type != VMDF_QCOW2)
+ break;
+
/*
* Clear the write and exclusive flags for base images.
* All writes should go to the top image, allowing them
@@ -354,16 +377,20 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, ui
*/
oflags = O_RDONLY | O_NONBLOCK;
aflags = R_OK;
- n = virtio_get_base(diskfds[i][j], base, sizeof(base),
- vmc->vmc_disktypes[i], path);
- if (n == 0)
- break;
+
+ /* Resolve the path of the next base image, if any. */
+ n = virtio_qcow2_get_base(diskfds[i][j], base,
+ sizeof(base), path);
if (n == -1) {
log_warnx("vm \"%s\" unable to read "
"base for disk %s", vmc->vmc_name,
vmc->vmc_disks[i]);
goto fail;
}
+ /* Are we at the last base image layer? */
+ if (n == 0)
+ break;
+
(void)strlcpy(path, base, sizeof(path));
}
}
blob - 2c03621a5123d7c656f23f545d2610d68e5a43af
blob + 3faa030f0a9de0047e72d3b8adbde01486c6a97f
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -88,8 +88,8 @@ int symset(const char *, const char *, int);
char *symget(const char *);
ssize_t parse_size(char *, int64_t);
-int parse_disk(char *, int);
-unsigned int parse_format(const char *);
+int parse_disk(char *, enum vm_disk_fmt);
+enum vm_disk_fmt parse_format(const char *);
static struct vmop_create_params vmc;
static struct vmd_switch *vsw;
@@ -104,6 +104,7 @@ extern const char *vmd_descsw[];
typedef struct {
union {
uint8_t lladdr[ETHER_ADDR_LEN];
+ enum vm_disk_fmt disk_format;
int64_t number;
char *string;
struct {
@@ -128,7 +129,7 @@ typedef struct {
%type <v.lladdr> lladdr
%type <v.number> bootdevice
%type <v.number> disable
-%type <v.number> image_format
+%type <v.disk_format> image_format
%type <v.number> local
%type <v.number> locked
%type <v.number> updown
@@ -642,10 +643,10 @@ agentxopts : /* none */
;
image_format : /* none */ {
- $$ = 0;
+ $$ = VMDF_AUTO;
}
| FORMAT string {
- if (($$ = parse_format($2)) == 0) {
+ if (($$ = parse_format($2)) == VMDF_INVALID) {
yyerror("unrecognized disk format %s", $2);
free($2);
YYERROR;
@@ -1355,11 +1356,9 @@ parse_size(char *word, int64_t val)
}
int
-parse_disk(char *word, int type)
+parse_disk(char *word, enum vm_disk_fmt type)
{
- char buf[BUFSIZ], path[PATH_MAX];
- int fd;
- ssize_t len;
+ char path[PATH_MAX];
if (vmc.vmc_ndisks >= VM_MAX_DISKS_PER_VM) {
log_warnx("too many disks");
@@ -1371,23 +1370,6 @@ parse_disk(char *word, int type)
return (-1);
}
- if (!type) {
- /* Use raw as the default format */
- type = VMDF_RAW;
-
- /* Try to derive the format from the file signature */
- if ((fd = open(path, O_RDONLY)) != -1) {
- len = read(fd, buf, sizeof(buf));
- close(fd);
- if (len >= (ssize_t)strlen(VM_MAGIC_QCOW) &&
- strncmp(buf, VM_MAGIC_QCOW,
- strlen(VM_MAGIC_QCOW)) == 0) {
- /* The qcow version will be checked later */
- type = VMDF_QCOW2;
- }
- }
- }
-
if (strlcpy(vmc.vmc_disks[vmc.vmc_ndisks], path,
sizeof(vmc.vmc_disks[vmc.vmc_ndisks])) >=
sizeof(vmc.vmc_disks[vmc.vmc_ndisks])) {
@@ -1401,14 +1383,14 @@ parse_disk(char *word, int type)
return (0);
}
-unsigned int
+enum vm_disk_fmt
parse_format(const char *word)
{
if (strcasecmp(word, "raw") == 0)
return (VMDF_RAW);
else if (strcasecmp(word, "qcow2") == 0)
return (VMDF_QCOW2);
- return (0);
+ return (VMDF_INVALID);
}
/*
blob - d7bee8e377aa22a7225b6573182b361ca0f018ed
blob + fed57a121fbb8251e811e980cbc40c9725c4f2bf
--- usr.sbin/vmd/vioblk.c
+++ usr.sbin/vmd/vioblk.c
@@ -36,7 +36,7 @@
extern struct vmd_vm *current_vm;
struct iovec io_v[VIRTIO_QUEUE_SIZE_MAX];
-static const char *disk_type(int);
+static const char *disk_type(enum vm_disk_fmt);
static uint32_t vioblk_read(struct virtio_dev *, struct viodev_msg *, int *);
static int vioblk_write(struct virtio_dev *, struct viodev_msg *);
static uint32_t vioblk_dev_read(struct virtio_dev *, struct viodev_msg *);
@@ -49,11 +49,13 @@ static void dev_dispatch_vm(int, short, void *);
static void handle_sync_io(int, short, void *);
static const char *
-disk_type(int type)
+disk_type(enum vm_disk_fmt type)
{
switch (type) {
+ case VMDF_AUTO: return "auto";
case VMDF_RAW: return "raw";
case VMDF_QCOW2: return "qcow2";
+ case VMDF_INVALID: return "invalid";
}
return "unknown";
}
@@ -67,7 +69,8 @@ vioblk_main(int fd, int fd_vmm)
struct vmd_vm vm;
ssize_t sz;
off_t szp = 0;
- int i, ret, type;
+ enum vm_disk_fmt type;
+ int i, ret;
/*
* stdio - needed for read/write to disk fds and channels to the vm.
blob - 662b74264004705438bd1ac464c6b5559c0544ec
blob + 9f3db23e18bcc2563723b013b55cd723c77e154b
--- usr.sbin/vmd/virtio.c
+++ usr.sbin/vmd/virtio.c
@@ -941,17 +941,18 @@ vmmci_io(int dir, uint16_t reg, uint32_t *data, uint8_
return (0);
}
-int
-virtio_get_base(int fd, char *path, size_t npath, int type, const char *dpath)
+enum vm_disk_fmt
+virtio_get_disktype(int fd)
{
- switch (type) {
- case VMDF_RAW:
- return 0;
- case VMDF_QCOW2:
- return virtio_qcow2_get_base(fd, path, npath, dpath);
- }
- log_warnx("%s: invalid disk format", __func__);
- return -1;
+ char buf[sizeof(VM_MAGIC_QCOW) - 1];
+ ssize_t len;
+
+ len = pread(fd, buf, sizeof(buf), 0);
+ if (len >= (ssize_t)sizeof(buf) &&
+ memcmp(buf, VM_MAGIC_QCOW, sizeof(buf)) == 0)
+ return (VMDF_QCOW2);
+
+ return (VMDF_RAW);
}
static void
blob - bfc32cec095e3a7ed218f4527058df284a467157
blob + 2ebc0b7bd8d460fa594afe4d829345ad3d81cb5f
--- usr.sbin/vmd/virtio.h
+++ usr.sbin/vmd/virtio.h
@@ -396,6 +396,7 @@ uint32_t virtio_io_cfg(struct virtio_dev *, int, uint8
void virtio_update_qs(struct virtio_dev *);
void virtio_update_qa(struct virtio_dev *);
+enum vm_disk_fmt virtio_get_disktype(int);
ssize_t virtio_qcow2_get_base(int, char *, size_t, const char *);
int virtio_qcow2_create(const char *, const char *, uint64_t);
int virtio_qcow2_init(struct virtio_backing *, off_t *, int*, size_t);
blob - d6bcfd05da4b5d22130c39573c0078d60a2f528e
blob + 3b3f5fb608dca3e8bcb44b201b2f8b7ef390d5e2
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -207,6 +207,13 @@ struct vmop_owner {
int64_t gid;
};
+enum vm_disk_fmt {
+ VMDF_INVALID = 0,
+ VMDF_AUTO,
+ VMDF_RAW,
+ VMDF_QCOW2,
+};
+
struct vmop_create_params {
/* vm identifying information */
uint32_t vmc_id;
@@ -248,10 +255,8 @@ struct vmop_create_params {
/* Emulated disk and cdrom drives */
size_t vmc_ndisks;
char vmc_disks[VM_MAX_DISKS_PER_VM][PATH_MAX];
- unsigned int vmc_disktypes[VM_MAX_DISKS_PER_VM];
+ enum vm_disk_fmt vmc_disktypes[VM_MAX_DISKS_PER_VM];
unsigned int vmc_diskbases[VM_MAX_DISKS_PER_VM];
-#define VMDF_RAW 0x01
-#define VMDF_QCOW2 0x02
char vmc_cdrom[PATH_MAX];
/* Emulated network devices */
@@ -581,9 +586,6 @@ int cmdline_symset(char *);
int parse_prefix4(const char *, struct local_prefix *, const char **);
int parse_prefix6(const char *, struct local_prefix *, const char **);
-/* virtio.c */
-int virtio_get_base(int, char *, size_t, int, const char *);
-
/* vionet.c */
__dead void vionet_main(int, int);
remove TOCTOU in vmd config and defer disk detection