Patchwork [v4,5/7] guest agent: add guest agent RPCs/commands

login
register
mail settings
Submitter Michael Roth
Date June 3, 2011, 10:48 p.m.
Message ID <1307141286-9392-6-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/98688/
State New
Headers show

Comments

Michael Roth - June 3, 2011, 10:48 p.m.
This adds the initial set of QMP/QAPI commands provided by the guest
agent:

guest-sync
guest-ping
guest-info
guest-file-open
guest-file-read
guest-file-write
guest-file-seek
guest-file-close
guest-fsfreeze-freeze
guest-fsfreeze-thaw
guest-fsfreeze-status

The input/output specification for these commands are documented in the
schema.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/guest-agent-commands.c |  522 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 522 insertions(+), 0 deletions(-)
 create mode 100644 qga/guest-agent-commands.c
Andi Kleen - June 4, 2011, 8:08 p.m.
Michael Roth <mdroth@linux.vnet.ibm.com> writes:
> +
> +int64_t qmp_guest_file_open(const char *filename, const char *mode, Error **err)
> +{
> +    FILE *fh;
> +    int fd, ret;
> +    int64_t id = -1;
> +
> +    if (!logging_enabled()) {
> +        error_set(err, QERR_QGA_LOGGING_FAILED);
> +        goto out;
> +    }
> +    slog("guest-file-open called, filename: %s, mode: %s", filename, mode);
> +    fh = fopen(filename, mode);
> +    if (!fh) {
> +        error_set(err, QERR_OPEN_FILE_FAILED, filename);
> +        goto out;
> +    }

Does this really allow a guest to open any host file ?!?
Have you considered all the security implications of that?

-Andi
Anthony Liguori - June 4, 2011, 10:29 p.m.
On 06/04/2011 03:08 PM, Andi Kleen wrote:
> Michael Roth<mdroth@linux.vnet.ibm.com>  writes:
>> +
>> +int64_t qmp_guest_file_open(const char *filename, const char *mode, Error **err)
>> +{
>> +    FILE *fh;
>> +    int fd, ret;
>> +    int64_t id = -1;
>> +
>> +    if (!logging_enabled()) {
>> +        error_set(err, QERR_QGA_LOGGING_FAILED);
>> +        goto out;
>> +    }
>> +    slog("guest-file-open called, filename: %s, mode: %s", filename, mode);
>> +    fh = fopen(filename, mode);
>> +    if (!fh) {
>> +        error_set(err, QERR_OPEN_FILE_FAILED, filename);
>> +        goto out;
>> +    }
>
> Does this really allow a guest to open any host file ?!?

It does the opposite.  The host can open files in the guest.  Since the 
host can see the disk image of the guest anyway, it already could do 
this albeit it in a more convoluted way.

Regards,

Anthony Liguroi

> Have you considered all the security implications of that?
>
> -Andi
>

Patch

diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
new file mode 100644
index 0000000..c836f33
--- /dev/null
+++ b/qga/guest-agent-commands.c
@@ -0,0 +1,522 @@ 
+/*
+ * QEMU Guest Agent commands
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <mntent.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <linux/fs.h>
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+
+static GAState *ga_state;
+
+static bool logging_enabled(void)
+{
+    return ga_logging_enabled(ga_state);
+}
+
+static void disable_logging(void)
+{
+    ga_disable_logging(ga_state);
+}
+
+static void enable_logging(void)
+{
+    ga_enable_logging(ga_state);
+}
+
+/* Note: in some situations, like with the fsfreeze, logging may be
+ * temporarilly disabled. if it is necessary that a command be able
+ * to log for accounting purposes, check logging_enabled() beforehand,
+ * and use the QERR_QGA_LOGGING_DISABLED to generate an error
+ */
+static void slog(const char *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
+    va_end(ap);
+}
+
+int64_t qmp_guest_sync(int64_t id, Error **errp)
+{
+    return id;
+}
+
+void qmp_guest_ping(Error **err)
+{
+    slog("guest-ping called");
+}
+
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+    GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+
+    info->version = g_strdup(QGA_VERSION);
+    info->timeout_ms = ga_get_timeout(ga_state);
+
+    return info;
+}
+
+void qmp_guest_shutdown(const char *shutdown_mode, Error **err)
+{
+    int ret;
+    const char *shutdown_flag;
+
+    if (!logging_enabled()) {
+        error_set(err, QERR_QGA_LOGGING_FAILED);
+        return;
+    }
+
+    slog("guest-shutdown called, shutdown_mode: %s", shutdown_mode);
+    if (strcmp(shutdown_mode, "halt") == 0) {
+        shutdown_flag = "-H";
+    } else if (strcmp(shutdown_mode, "powerdown") == 0) {
+        shutdown_flag = "-P";
+    } else if (strcmp(shutdown_mode, "reboot") == 0) {
+        shutdown_flag = "-r";
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER_VALUE, "shutdown_mode",
+                  "halt|powerdown|reboot");
+        return;
+    }
+
+    ret = fork();
+    if (ret == 0) {
+        /* child, start the shutdown */
+        setsid();
+        fclose(stdin);
+        fclose(stdout);
+        fclose(stderr);
+
+        sleep(5);
+        ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
+                    "hypervisor initiated shutdown", (char*)NULL);
+        exit(!!ret);
+    } else if (ret < 0) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+    }
+}
+
+typedef struct GuestFileHandle {
+    uint64_t id;
+    FILE *fh;
+} GuestFileHandle;
+
+static struct {
+    GSList *filehandles;
+    uint64_t last_id;
+} guest_file_state;
+
+static int64_t guest_file_handle_add(FILE *fh)
+{
+    GuestFileHandle *gfh;
+
+    gfh = g_malloc(sizeof(GuestFileHandle));
+    gfh->id = guest_file_state.last_id++;
+    gfh->fh = fh;
+    guest_file_state.filehandles = g_slist_append(guest_file_state.filehandles,
+                                                  gfh);
+    return gfh->id;
+}
+
+static gint guest_file_handle_match(gconstpointer elem, gconstpointer id_p)
+{
+    const uint64_t *id = id_p;
+    const GuestFileHandle *gfh = elem;
+
+    g_assert(gfh);
+    return (gfh->id != *id);
+}
+
+static FILE *guest_file_handle_find(int64_t id)
+{
+    GSList *elem = g_slist_find_custom(guest_file_state.filehandles, &id,
+                                       guest_file_handle_match);
+    GuestFileHandle *gfh;
+
+    if (elem) {
+        g_assert(elem->data);
+        gfh = elem->data;
+        return gfh->fh;
+    }
+
+    return NULL;
+}
+
+static void guest_file_handle_remove(int64_t id)
+{
+    GSList *elem = g_slist_find_custom(guest_file_state.filehandles, &id,
+                                       guest_file_handle_match);
+    gpointer data = elem->data;
+
+    if (!data) {
+        return;
+    }
+    guest_file_state.filehandles = g_slist_remove(guest_file_state.filehandles,
+                                                  data);
+    g_free(data);
+}
+
+int64_t qmp_guest_file_open(const char *filename, const char *mode, Error **err)
+{
+    FILE *fh;
+    int fd, ret;
+    int64_t id = -1;
+
+    if (!logging_enabled()) {
+        error_set(err, QERR_QGA_LOGGING_FAILED);
+        goto out;
+    }
+    slog("guest-file-open called, filename: %s, mode: %s", filename, mode);
+    fh = fopen(filename, mode);
+    if (!fh) {
+        error_set(err, QERR_OPEN_FILE_FAILED, filename);
+        goto out;
+    }
+
+    /* set fd non-blocking to avoid common use cases (like reading from a
+     * named pipe) from hanging the agent
+     */
+    fd = fileno(fh);
+    ret = fcntl(fd, F_GETFL);
+    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
+    if (ret == -1) {
+        error_set(err, QERR_OPEN_FILE_FAILED, filename);
+        fclose(fh);
+        goto out;
+    }
+
+    id = guest_file_handle_add(fh);
+    slog("guest-file-open, filehandle: %ld", id);
+out:
+    return id;
+}
+
+struct GuestFileRead *qmp_guest_file_read(int64_t filehandle, int64_t count,
+                                          Error **err)
+{
+    GuestFileRead *read_data;
+    guchar *buf;
+    FILE *fh = guest_file_handle_find(filehandle);
+    size_t read_count;
+
+    if (!fh) {
+        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
+        return NULL;
+    }
+
+    read_data = g_malloc0(sizeof(GuestFileRead));
+    buf = g_malloc(count);
+
+    read_count = fread(buf, 1, count, fh);
+    buf[read_count] = 0;
+    read_data->count = read_count;
+    read_data->eof = feof(fh);
+    if (read_count) {
+        read_data->buf = g_base64_encode(buf, read_count);
+    }
+    g_free(buf);
+    /* clear error and eof. error is generally due to EAGAIN from non-blocking
+     * mode, and no real way to differenitate from a real error since we only
+     * get boolean error flag from ferror()
+     */
+    clearerr(fh);
+
+    return read_data;
+}
+
+GuestFileWrite *qmp_guest_file_write(int64_t filehandle, const char *data_b64,
+                                     int64_t count, Error **err)
+{
+    GuestFileWrite *write_data;
+    guchar *data;
+    gsize data_len;
+    int write_count;
+    FILE *fh = guest_file_handle_find(filehandle);
+
+    if (!fh) {
+        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
+        return NULL;
+    }
+
+    write_data = g_malloc0(sizeof(GuestFileWrite));
+    data = g_base64_decode(data_b64, &data_len);
+    write_count = fwrite(data, 1, MIN(count, data_len), fh);
+    write_data->count = write_count;
+    write_data->eof = feof(fh);
+    g_free(data);
+    clearerr(fh);
+
+    return write_data;
+}
+
+struct GuestFileSeek *qmp_guest_file_seek(int64_t filehandle, int64_t offset,
+                                          int64_t whence, Error **err)
+{
+    GuestFileSeek *seek_data;
+    FILE *fh = guest_file_handle_find(filehandle);
+    int ret;
+
+    if (!fh) {
+        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
+        return NULL;
+    }
+
+    seek_data = g_malloc0(sizeof(GuestFileRead));
+    ret = fseek(fh, offset, whence);
+    if (ret == -1) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+        g_free(seek_data);
+        return NULL;
+    }
+    seek_data->position = ftell(fh);
+    seek_data->eof = feof(fh);
+    clearerr(fh);
+
+    return seek_data;
+}
+
+void qmp_guest_file_close(int64_t filehandle, Error **err)
+{
+    FILE *fh = guest_file_handle_find(filehandle);
+
+    if (!logging_enabled()) {
+        error_set(err, QERR_QGA_LOGGING_FAILED);
+        return;
+    }
+    slog("guest-file-close called, filehandle: %ld", filehandle);
+    if (!fh) {
+        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
+        return;
+    }
+
+    fclose(fh);
+    guest_file_handle_remove(filehandle);
+}
+
+/*
+ * Walk the mount table and build a list of local file systems
+ */
+
+struct direntry {
+    char *dirname;
+    char *devtype;
+    struct direntry *next;
+};
+
+struct {
+    struct direntry *mount_list;
+    enum {
+        FREEZE_ERROR = -1,
+        FREEZE_THAWED = 0,
+        FREEZE_INPROGRESS = 1,
+        FREEZE_FROZEN = 2,
+    } status;
+} guest_fsfreeze_state;
+
+static int guest_fsfreeze_build_mount_list(void)
+{
+    struct mntent *mnt;
+    struct direntry *entry;
+    struct direntry *next;
+    char const *mtab = MOUNTED;
+    FILE *fp;
+
+    fp = setmntent(mtab, "r");
+    if (!fp) {
+        g_warning("fsfreeze: unable to read mtab");
+        goto fail;
+    }
+
+    while ((mnt = getmntent(fp))) {
+        /*
+         * An entry which device name doesn't start with a '/' is
+         * either a dummy file system or a network file system.
+         * Add special handling for smbfs and cifs as is done by
+         * coreutils as well.
+         */
+        if ((mnt->mnt_fsname[0] != '/') ||
+            (strcmp(mnt->mnt_type, "smbfs") == 0) ||
+            (strcmp(mnt->mnt_type, "cifs") == 0)) {
+            continue;
+        }
+
+        entry = g_malloc(sizeof(struct direntry));
+        entry->dirname = qemu_strdup(mnt->mnt_dir);
+        entry->devtype = qemu_strdup(mnt->mnt_type);
+        entry->next = guest_fsfreeze_state.mount_list;
+
+        guest_fsfreeze_state.mount_list = entry;
+    }
+
+    endmntent(fp);
+
+    return 0;
+
+fail:
+    while(guest_fsfreeze_state.mount_list) {
+        next = guest_fsfreeze_state.mount_list->next;
+        g_free(guest_fsfreeze_state.mount_list->dirname);
+        g_free(guest_fsfreeze_state.mount_list->devtype);
+        g_free(guest_fsfreeze_state.mount_list);
+        guest_fsfreeze_state.mount_list = next;
+    }
+
+    return -1;
+}
+
+/* 
+ * Return status of freeze/thaw
+ */
+int64_t qmp_guest_fsfreeze_status(Error **err)
+{
+    return guest_fsfreeze_state.status;
+}
+
+/*
+ * Walk list of mounted file systems in the guest, and freeze the ones which
+ * are real local file systems.
+ */
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
+{
+    int ret = 0, i = 0;
+    struct direntry *entry;
+    int fd;
+
+    if (!logging_enabled()) {
+        error_set(err, QERR_QGA_LOGGING_FAILED);
+        goto out;
+    }
+
+    slog("guest-fsfreeze called");
+
+    if (guest_fsfreeze_state.status != FREEZE_THAWED) {
+        ret = 0;
+        goto out;
+    }
+
+    ret = guest_fsfreeze_build_mount_list();
+    if (ret < 0) {
+        goto out;
+    }
+
+    guest_fsfreeze_state.status = FREEZE_INPROGRESS;
+
+    /* cannot risk guest agent blocking itself on a write in this state */
+    disable_logging();
+
+    entry = guest_fsfreeze_state.mount_list;
+    while(entry) {
+        fd = qemu_open(entry->dirname, O_RDONLY);
+        if (fd == -1) {
+            ret = errno;
+            goto error;
+        }
+
+        /* we try to cull filesytems we know won't work in advance, but other
+         * filesytems may not implement fsfreeze for less obvious reasons.
+         * these will reason EOPNOTSUPP, so we simply ignore them. when
+         * thawing, these filesystems will return an EINVAL instead, due to
+         * not being in a frozen state. Other filesystem-specific
+         * errors may result in EINVAL, however, so the user should check the
+         * number * of filesystems returned here against those returned by the
+         * thaw operation to determine whether everything completed
+         * successfully
+         */
+        ret = ioctl(fd, FIFREEZE);
+        if (ret < 0 && errno != EOPNOTSUPP) {
+            close(fd);
+            goto error;
+        }
+        close(fd);
+
+        entry = entry->next;
+        i++;
+    }
+
+    guest_fsfreeze_state.status = FREEZE_FROZEN;
+    ret = i;
+out:
+    return ret;
+error:
+    if (i > 0) {
+        guest_fsfreeze_state.status = FREEZE_ERROR;
+    }
+    goto out;
+}
+
+/*
+ * Walk list of frozen file systems in the guest, and thaw them.
+ */
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
+{
+    int ret;
+    struct direntry *entry;
+    int fd, i = 0;
+
+    if (guest_fsfreeze_state.status != FREEZE_FROZEN &&
+        guest_fsfreeze_state.status != FREEZE_INPROGRESS) {
+        ret = 0;
+        goto out_enable_logging;
+    }
+
+    while((entry = guest_fsfreeze_state.mount_list)) {
+        fd = qemu_open(entry->dirname, O_RDONLY);
+        if (fd == -1) {
+            ret = -errno;
+            goto out;
+        }
+        ret = ioctl(fd, FITHAW);
+        if (ret < 0 && errno != EOPNOTSUPP && errno != EINVAL) {
+            ret = -errno;
+            close(fd);
+            goto out;
+        }
+        close(fd);
+
+        guest_fsfreeze_state.mount_list = entry->next;
+        g_free(entry->dirname);
+        g_free(entry->devtype);
+        g_free(entry);
+        i++;
+    }
+
+    guest_fsfreeze_state.status = FREEZE_THAWED;
+    ret = i;
+out_enable_logging:
+    enable_logging();
+out:
+    return ret;
+}
+
+static void guest_fsfreeze_cleanup(void)
+{
+    int64_t ret;
+    Error *err = NULL;
+
+    if (guest_fsfreeze_state.status == FREEZE_FROZEN) {
+        ret = qmp_guest_fsfreeze_thaw(&err);
+        if (ret < 0 || err) {
+            slog("failed to clean up frozen filesystems");
+        }
+    }
+}
+
+/* register init/cleanup routines for stateful command groups */
+void ga_command_state_init(GAState *s, GACommandState *cs)
+{
+    ga_state = s;
+    ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
+}