From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60627 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PQnnX-0007uR-39 for qemu-devel@nongnu.org; Thu, 09 Dec 2010 16:13:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PQnnT-0007yt-Cc for qemu-devel@nongnu.org; Thu, 09 Dec 2010 16:13:06 -0500 Received: from e2.ny.us.ibm.com ([32.97.182.142]:48316) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PQnnT-0007yf-53 for qemu-devel@nongnu.org; Thu, 09 Dec 2010 16:13:03 -0500 Received: from d01dlp02.pok.ibm.com (d01dlp02.pok.ibm.com [9.56.224.85]) by e2.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id oB9KuP7e029434 for ; Thu, 9 Dec 2010 15:56:25 -0500 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id A31AD4DE8045 for ; Thu, 9 Dec 2010 16:11:06 -0500 (EST) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oB9LD1kf230502 for ; Thu, 9 Dec 2010 16:13:01 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oB9LD06r030722 for ; Thu, 9 Dec 2010 16:13:01 -0500 Message-ID: <4D01465B.9040200@linux.vnet.ibm.com> Date: Thu, 09 Dec 2010 15:12:59 -0600 From: Michael Roth MIME-Version: 1.0 References: <1291399402-20366-1-git-send-email-mdroth@linux.vnet.ibm.com> <1291399402-20366-9-git-send-email-mdroth@linux.vnet.ibm.com> <4CFE4406.4010209@redhat.com> In-Reply-To: <4CFE4406.4010209@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, abeekhof@redhat.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com 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 >> --- >> 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 >> +@item agent_viewfile @var{filepath} >> +@findex 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 >> +@item agent_viewfile @var{filepath} >> +@findex 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 >