qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp


From: Michael Roth
Subject: [Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command
Date: Thu, 09 Dec 2010 15:12:59 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.12) Gecko/20101027 Thunderbird/3.1.6

On 12/07/2010 08:26 AM, Jes Sorensen wrote:
On 12/03/10 19:03, Michael Roth wrote:
Utilize the getfile RPC to provide a means to view text files in the
guest. Getfile can handle binary files as well but we don't advertise
that here due to the special handling requiring to store it and provide
it back to the user (base64 encoding it for instance). Hence the
otherwise confusing "viewfile" as opposed to "getfile".

Signed-off-by: Michael Roth<address@hidden>
---
  hmp-commands.hx |   16 +++++++++
  monitor.c       |    1 +
  qmp-commands.hx |   33 +++++++++++++++++++
  virtagent.c     |   96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
  virtagent.h     |    3 ++
  5 files changed, 149 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e5585ba..423c752 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1212,6 +1212,22 @@ show available trace events and their state
  ETEXI
  #endif

+    {
+        .name       = "agent_viewfile",
+        .args_type  = "filepath:s",
+        .params     = "filepath",
+        .help       = "Echo a file from the guest filesystem",
+        .user_print = do_agent_viewfile_print,
+        .mhandler.cmd_async = do_agent_viewfile,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
address@hidden agent_viewfile @var{filepath}
address@hidden agent_viewfile
+Echo the file identified by @var{filepath} on the guest filesystem
+ETEXI
+
  STEXI
  @end table
  ETEXI
diff --git a/monitor.c b/monitor.c
index 8cee35d..145895d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -56,6 +56,7 @@
  #include "json-parser.h"
  #include "osdep.h"
  #include "exec-all.h"
+#include "virtagent.h"
  #ifdef CONFIG_SIMPLE_TRACE
  #include "trace.h"
  #endif
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 793cf1c..efa2137 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -738,6 +738,39 @@ Example:
  EQMP

      {
+        .name       = "agent_viewfile",
+        .args_type  = "filepath:s",
+        .params     = "filepath",
+        .help       = "Echo a file from the guest filesystem",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_async = do_agent_viewfile,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
address@hidden agent_viewfile @var{filepath}
address@hidden agent_viewfile
+Echo the file identified by @var{filepath} on the guest filesystem
+ETEXI
+SQMP
+agent_viewfile
+--------
+
+Echo the file identified by @var{filepath} from the guest filesystem.
+
+Arguments:
+
+- "filepath": Full guest path of the desired file
+
+Example:
+
+->  { "execute": "agent_viewfile",
+                "arguments": { "filepath": "/sys/kernel/kexec_loaded" } }
+<- { "return": { "contents": "0" } }
+
+EQMP
+
+    {
          .name       = "qmp_capabilities",
          .args_type  = "",
          .params     = "",
diff --git a/virtagent.c b/virtagent.c
index 34d8545..4a4dc8a 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -139,3 +139,99 @@ out_free:
  out:
      return ret;
  }
+
+/* QMP/HMP RPC client functions */
+
+void do_agent_viewfile_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+    const char *contents = NULL;
+    int i;
+
+    qdict = qobject_to_qdict(data);
+    if (!qdict_haskey(qdict, "contents")) {
+        return;
+    }
+
+    contents = qdict_get_str(qdict, "contents");
+    if (contents != NULL) {
+         /* monitor_printf truncates so do it in chunks. also, file_contents
+          * may not be null-termed at proper location so explicitly calc
+          * last chunk sizes */
+        for (i = 0; i<  strlen(contents); i += 1024) {
+            monitor_printf(mon, "%.1024s", contents + i);
+        }
+    }
+    monitor_printf(mon, "\n");
+}
+
+static void do_agent_viewfile_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    char *file_contents = NULL;
+    size_t file_size;
+    int ret;
+    xmlrpc_env env;
+    QDict *qdict = qdict_new();
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        goto out_no_resp;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        ret = -1;
+        goto out_no_resp;
+    }
+
+    xmlrpc_parse_value(&env, resp, "6",&file_contents,&file_size);
+    if (va_rpc_has_error(&env)) {
+        ret = -1;
+        goto out;

I believe this suffers from the same architectural problem I mentioned
in my comment to 07/21 - you don't restrict the file size, so it could
blow up the QEMU process on the host trying to view the wrong file.

It's restricted on the guest side:

virtagent-server.c:va_getfile():

    while ((ret = read(fd, buf, VA_FILEBUF_LEN)) > 0) {
file_contents = qemu_realloc(file_contents, count + VA_FILEBUF_LEN);
        memcpy(file_contents + count, buf, ret);
        count += ret;
        if (count > VA_GETFILE_MAX) {
            xmlrpc_faultf(env, "max file size (%d bytes) exceeded",
                          VA_GETFILE_MAX);
            goto EXIT_CLOSE_BAD;
        }
    }

There are additional limits at the transport layer well to deal with a potentially malicious/buggy agent as well:

virtagent-common.c:va_http_read_handler():

            } else if (s->content_len > VA_CONTENT_LEN_MAX) {
                LOG("http content length too long");
                goto out_bad;
            }

And configurable limits enforced by the xmlrpc-c library on the host and guest side, which I haven't been explicitly setting or keeping consistent with the other various limits. I'll address this for the next round.


I really think it is a bad idea to put this kind of command into the
monitor.

Jes





reply via email to

[Prev in Thread] Current Thread [Next in Thread]