diff mbox

[for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU

Message ID 1464276116-8412-1-git-send-email-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu May 26, 2016, 3:21 p.m. UTC 
XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We
explicitly set the limit in libxl for 4.7.

Introduce a function for setting the environment variable and call it in
the right places.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dm.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

Comments

Ian Jackson May 26, 2016, 3:41 p.m. UTC | #1 
Wei Liu writes ("[PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU"):
> XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We
> explicitly set the limit in libxl for 4.7.
...
> +    unsigned long limit = 0;
> +    const char *s = getenv("XEN_QEMU_CONSOLE_LIMIT");
> +
> +    limit = s ? strtoul(s,0,0) : 1*1024*1024;
> +    flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT",
> +                          GCSPRINTF("%lu", limit));

This is rather more complex than it needs to be.  What would be wrong
with
  if (getenv("XEN_QEMU_CONSOLE_LIMIT")) return;
  flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT", "1048576");
?

But I guess I don't object enough to care, so

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>


> +    flexarray_t *dm_args, *dm_envs;
> +    char **args, **envs;

All these parts are a bit of a palaver but I don't see a better way.
Thanks for doing this work!

Ian.
Wei Liu May 26, 2016, 3:47 p.m. UTC | #2 
On Thu, May 26, 2016 at 04:41:53PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU"):
> > XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We
> > explicitly set the limit in libxl for 4.7.
> ...
> > +    unsigned long limit = 0;
> > +    const char *s = getenv("XEN_QEMU_CONSOLE_LIMIT");
> > +
> > +    limit = s ? strtoul(s,0,0) : 1*1024*1024;
> > +    flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT",
> > +                          GCSPRINTF("%lu", limit));
> 
> This is rather more complex than it needs to be.  What would be wrong
> with
>   if (getenv("XEN_QEMU_CONSOLE_LIMIT")) return;
>   flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT", "1048576");
> ?
> 

Nice, this is simpler. I will use this version and keep your ack.

> But I guess I don't object enough to care, so
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> 
> > +    flexarray_t *dm_args, *dm_envs;
> > +    char **args, **envs;
> 
> All these parts are a bit of a palaver but I don't see a better way.
> Thanks for doing this work!
> 

YW.

Wei.

> Ian.
diff mbox

Patch

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 6bbc7c3..1412c29 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -365,6 +365,24 @@  int libxl__domain_device_construct_rdm(libxl__gc *gc,
     return ERROR_FAIL;
 }
 
+/* XSA-180 / CVE-2014-3672
+ *
+ * The QEMU shipped with Xen has a bodge. It checks for
+ * XEN_QEMU_CONSOLE_LIMIT to see how much data QEMU is allowed
+ * to write to stderr. We set that to 1MB if it is not set by
+ * system administrator.
+ */
+static void libxl__set_qemu_env_for_xsa_180(libxl__gc *gc,
+                                            flexarray_t *dm_envs)
+{
+    unsigned long limit = 0;
+    const char *s = getenv("XEN_QEMU_CONSOLE_LIMIT");
+
+    limit = s ? strtoul(s,0,0) : 1*1024*1024;
+    flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT",
+                          GCSPRINTF("%lu", limit));
+}
+
 const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config *guest_config)
 {
     const libxl_vnc_info *vnc = NULL;
@@ -415,6 +433,8 @@  static int libxl__build_device_model_args_old(libxl__gc *gc,
     dm_args = flexarray_make(gc, 16, 1);
     dm_envs = flexarray_make(gc, 16, 1);
 
+    libxl__set_qemu_env_for_xsa_180(gc, dm_envs);
+
     flexarray_vappend(dm_args, dm,
                       "-d", GCSPRINTF("%d", domid), NULL);
 
@@ -913,6 +933,8 @@  static int libxl__build_device_model_args_new(libxl__gc *gc,
     dm_args = flexarray_make(gc, 16, 1);
     dm_envs = flexarray_make(gc, 16, 1);
 
+    libxl__set_qemu_env_for_xsa_180(gc, dm_envs);
+
     flexarray_vappend(dm_args, dm,
                       "-xen-domid",
                       GCSPRINTF("%d", guest_domid), NULL);
@@ -2193,8 +2215,8 @@  static void device_model_spawn_outcome(libxl__egc *egc,
 void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
 {
     STATE_AO_GC(dmss->spawn.ao);
-    flexarray_t *dm_args;
-    char **args;
+    flexarray_t *dm_args, *dm_envs;
+    char **args, **envs;
     const char *dm;
     int logfile_w, null = -1, rc;
     uint32_t domid = dmss->guest_domid;
@@ -2203,6 +2225,8 @@  void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
     dm = qemu_xen_path(gc);
 
     dm_args = flexarray_make(gc, 15, 1);
+    dm_envs = flexarray_make(gc, 1, 1);
+
     flexarray_vappend(dm_args, dm, "-xen-domid",
                       GCSPRINTF("%d", domid), NULL);
     flexarray_append(dm_args, "-xen-attach");
@@ -2216,6 +2240,9 @@  void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
     flexarray_append(dm_args, NULL);
     args = (char **) flexarray_contents(dm_args);
 
+    libxl__set_qemu_env_for_xsa_180(gc, dm_envs);
+    envs = (char **) flexarray_contents(dm_envs);
+
     logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qdisk-%u", domid));
     if (logfile_w < 0) {
         rc = logfile_w;
@@ -2253,7 +2280,7 @@  void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
         goto out;
     if (!rc) { /* inner child */
         setsid();
-        libxl__exec(gc, null, logfile_w, logfile_w, dm, args, NULL);
+        libxl__exec(gc, null, logfile_w, logfile_w, dm, args, envs);
     }
 
     rc = 0;