Skip to contentSkip to conversation

XSA-180 patches for 4.7

PATCH New Wei Liu 4 participants 11 messages May 26, 2016

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/1 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.

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

---
 tools/libxl/libxl_dm.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)
tools/libxl/libxl_dm.c +30 -3
index 6bbc7c3..1412c29 100644
@@ -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;
Signature · 7 lines
2.1.4


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
author
#2 lore raw

PATCH 1/1 main loop: Big hammer to fix logfile disk DoS in Xen setups

From: Ian Jackson <[email protected]>

Each time round the main loop, we now fstat stderr. If it is too big, we dup2 /dev/null onto it. This is not a very pretty patch but it is very simple, easy to see that it's correct, and has a low risk of collateral damage.

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

This fixes CVE-2014-3672.

Set the default to 0 so that it won't affect non-xen installation. The limit will be set by Xen toolstack.

---
 main-loop.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
main-loop.c +48 -0
index 3997043..aa32f5b 100644
@@ -164,6 +164,50 @@ int qemu_init_main_loop(Error **errp)
     return 0;
 }
 
+static void check_cve_2014_3672_xen(void)
+{
+    static unsigned long limit = ~0UL;
+    const int fd = 2;
+    struct stat stab;
+
+    if (limit == ~0UL) {
+        const char *s = getenv("XEN_QEMU_CONSOLE_LIMIT");
+        /* XEN_QEMU_CONSOLE_LIMIT=0 means no limit */
+        limit = s ? strtoul(s,0,0) : 0;
+    }
+    if (limit == 0)
+        return;
+
+    int r = fstat(fd, &stab);
+    if (r) {
+        perror("fstat stderr (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    if (!S_ISREG(stab.st_mode))
+        return;
+    if (stab.st_size <= limit)
+        return;
+
+    /* oh dear */
+    fprintf(stderr,"\r\n"
+            "Closing stderr due to CVE-2014-3672 limit. "
+            " Set XEN_QEMU_CONSOLE_LIMIT to number of bytes to override,"
+            " or 0 for no limit.\n");
+    fflush(stderr);
+
+    int nfd = open("/dev/null", O_WRONLY);
+    if (nfd < 0) {
+        perror("open /dev/null (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    r = dup2(nfd, fd);
+    if (r != fd) {
+        perror("dup2 /dev/null (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    close(nfd);
+}
+
 static int max_priority;
 
 #ifndef _WIN32
@@ -216,6 +260,8 @@ static int os_host_main_loop_wait(int64_t timeout)
     int ret;
     static int spin_counter;
 
+    check_cve_2014_3672_xen();
+
     glib_pollfds_fill(&timeout);
 
     /* If the I/O thread is very busy or we are incorrectly busy waiting in
@@ -407,6 +453,8 @@ static int os_host_main_loop_wait(int64_t timeout)
     fd_set rfds, wfds, xfds;
     int nfds;
 
+    check_cve_2014_3672_xen();
+
     /* XXX: need to suppress polling by better using win32 events */
     ret = 0;
     for (pe = first_polling_entry; pe != NULL; pe = pe->next) {
Signature · 7 lines
2.1.4


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Acked-by on v1 1/1
#3 lore raw

Re: [PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups

Wei Liu writes ("[PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups"):

Quoted 10 lines

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

This fixes CVE-2014-3672.

Set the default to 0 so that it won't affect non-xen installation. The limit will be set by Xen toolstack.

Maybe the commit message, earlier, could be adjusted ? But otherwise

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#4 lore raw

Re: [PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups

In reply to Wei Liu
Quoted 9 lines · Wei Liu

From: Ian Jackson <[email protected]>

Each time round the main loop, we now fstat stderr. If it is too big, we dup2 /dev/null onto it. This is not a very pretty patch but it is very simple, easy to see that it's correct, and has a low risk of collateral damage.

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

There is no limit by default; a companion patch for libxl sets this to 1MiB. This is so that this patch may be applied on a system qemu which is shared between multiple use cases (including non-Xen cases).

-George

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

author
#5 lore raw

Re: [PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups

Quoted 8 lines · George Dunlap
Quoted 7 lines · Wei Liu

From: Ian Jackson <[email protected]>

Each time round the main loop, we now fstat stderr. If it is too big, we dup2 /dev/null onto it. This is not a very pretty patch but it is very simple, easy to see that it's correct, and has a low risk of collateral damage.

Quoted 7 lines
Quoted 2 lines

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

There is no limit by default; a companion patch for libxl sets this to 1MiB. This is so that this patch may be applied on a system qemu which is shared between multiple use cases (including non-Xen cases).

I added a paragraph in that patch and have my S-o-B.

Did you miss that? Or do you mean I should rewrite the commit message altogether?

Wei.

Quoted 1 line

-George

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Acked-by on v1 1/1
#6 lore raw

Re: [PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups

In reply to Wei Liu
Quoted 12 lines · Wei Liu

From: Ian Jackson <[email protected]>

Each time round the main loop, we now fstat stderr. If it is too big, we dup2 /dev/null onto it. This is not a very pretty patch but it is very simple, easy to see that it's correct, and has a low risk of collateral damage.

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

This fixes CVE-2014-3672.

Quoted 7 lines

Set the default to 0 so that it won't affect non-xen installation. The limit will be set by Xen toolstack.

Beside the confusing commit message and the call in the WIN32 specific mainloop, the patch look good. So,

Quoted 69 lines
---
 main-loop.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
main-loop.c +46 -0
index 3997043..aa32f5b 100644
@@ -164,6 +164,50 @@ int qemu_init_main_loop(Error **errp)
     return 0;
 }
 
+static void check_cve_2014_3672_xen(void)
+{
+    static unsigned long limit = ~0UL;
+    const int fd = 2;
+    struct stat stab;
+
+    if (limit == ~0UL) {
+        const char *s = getenv("XEN_QEMU_CONSOLE_LIMIT");
+        /* XEN_QEMU_CONSOLE_LIMIT=0 means no limit */
+        limit = s ? strtoul(s,0,0) : 0;
+    }
+    if (limit == 0)
+        return;
+
+    int r = fstat(fd, &stab);
+    if (r) {
+        perror("fstat stderr (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    if (!S_ISREG(stab.st_mode))
+        return;
+    if (stab.st_size <= limit)
+        return;
+
+    /* oh dear */
+    fprintf(stderr,"\r\n"
+            "Closing stderr due to CVE-2014-3672 limit. "
+            " Set XEN_QEMU_CONSOLE_LIMIT to number of bytes to override,"
+            " or 0 for no limit.\n");
+    fflush(stderr);
+
+    int nfd = open("/dev/null", O_WRONLY);
+    if (nfd < 0) {
+        perror("open /dev/null (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    r = dup2(nfd, fd);
+    if (r != fd) {
+        perror("dup2 /dev/null (for CVE-2014-3672 check)");
+        exit(-1);
+    }
+    close(nfd);
+}
+
 static int max_priority;
 
 #ifndef _WIN32
@@ -216,6 +260,8 @@ static int os_host_main_loop_wait(int64_t timeout)
     int ret;
     static int spin_counter;
 
+    check_cve_2014_3672_xen();
+
     glib_pollfds_fill(&timeout);
 
     /* If the I/O thread is very busy or we are incorrectly busy waiting in

main-loop.c static int os_host_main_loop_wait(int64_t timeout) · line 457

Quoted 5 lines
main-loop.c +2 -0
@@ -407,6 +453,8 @@ static int os_host_main_loop_wait(int64_t timeout)
     fd_set rfds, wfds, xfds;
     int nfds;
 
+    check_cve_2014_3672_xen();
+

That's the call within an #ifdef _WIN32.

Quoted 3 lines
main-loop.c +0 -0
     /* XXX: need to suppress polling by better using win32 events */
     ret = 0;
     for (pe = first_polling_entry; pe != NULL; pe = pe->next) {
Signature · 6 lines
Anthony PERARD

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
#7 lore raw

Re: [PATCH QEMU for-4.7] main loop: Big hammer to fix logfile disk DoS in Xen setups

Quoted 19 lines · Anthony PERARD
Quoted 18 lines · Wei Liu

From: Ian Jackson <[email protected]>

Each time round the main loop, we now fstat stderr. If it is too big, we dup2 /dev/null onto it. This is not a very pretty patch but it is very simple, easy to see that it's correct, and has a low risk of collateral damage.

The limit is 1Mby by default but can be adjusted by setting a new environment variable.

This fixes CVE-2014-3672.

Set the default to 0 so that it won't affect non-xen installation. The limit will be set by Xen toolstack.

Quoted 6 lines
Quoted 1 line

Beside the confusing commit message and the call in the WIN32 specific mainloop, the patch look good. So,

I have ajusted the commit message with
s/The limit is 1Mby/There is no limit/ and going to push to staging.

Signature · 6 lines
Anthony PERARD

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Acked-by on v1 1/1
#8 lore raw

Re: [PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU

In reply to Wei Liu

Wei Liu writes ("[PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU"):

Quoted 2 lines

XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We explicitly set the limit in libxl for 4.7.

...

tools/libxl/libxl_dm.c int libxl__domain_device_construct_rdm(libxl__gc *gc, · line 383

Quoted 6 lines
tools/libxl/libxl_dm.c +6 -0
+    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

tools/libxl/libxl_dm.c static void device_model_spawn_outcome(libxl__egc *egc, · line 2219

Quoted 2 lines
tools/libxl/libxl_dm.c +2 -0
+    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.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

author
#9 lore raw

Re: [PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU

Quoted 11 lines · Ian Jackson

Wei Liu writes ("[PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU"):

Quoted 2 lines

XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We explicitly set the limit in libxl for 4.7.

...

tools/libxl/libxl_dm.c int libxl__domain_device_construct_rdm(libxl__gc *gc, · line 383

Quoted 6 lines
tools/libxl/libxl_dm.c +6 -0
+    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));
Quoted 6 lines

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.

Quoted 11 lines

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

tools/libxl/libxl_dm.c static void device_model_spawn_outcome(libxl__egc *egc, · line 2219

Quoted 2 lines
tools/libxl/libxl_dm.c +2 -0
+    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.

Quoted 1 line

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

author Acked-by Ian Jackson
#10 lore raw

Re: [PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU

Quoted 12 lines · Wei Liu
Quoted 11 lines · Ian Jackson

Wei Liu writes ("[PATCH for-4.7] libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU"):

Quoted 2 lines

XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We explicitly set the limit in libxl for 4.7.

...

Quoted 6 lines
(quoted diff) +6 -0
+    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));
Quoted 9 lines
Quoted 6 lines

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.

I will push this patch soon.

---8<---
From 1d915bc7805bde9fe90341e9bcea01bf50154d87 Mon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Thu, 26 May 2016 16:11:42 +0100
Subject: [PATCH] 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.

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

---
 tools/libxl/libxl_dm.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
tools/libxl/libxl_dm.c +26 -3
index 6bbc7c3..155a653 100644
@@ -365,6 +365,20 @@ 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)
+{
+    if (getenv("XEN_QEMU_CONSOLE_LIMIT")) return;
+    flexarray_append_pair(dm_envs, "XEN_QEMU_CONSOLE_LIMIT", "1048576");
+}
+
 const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config *guest_config)
 {
     const libxl_vnc_info *vnc = NULL;
@@ -415,6 +429,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 +929,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 +2211,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 +2221,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 +2236,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 +2276,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;
Signature · 8 lines
2.1.4



_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel