libxl: Fix nul-termination of the return value of libxl_xen_console_read_line()

When built with ASAN, "xl dmesg" crashes in the "printf("%s", line)"
call in main_dmesg().  ASAN reports a heap buffer overflow: an
off-by-one access to cr->buffer.

The readconsole sysctl copies up to count characters into the buffer,
but it does not add a null character at the end.  Despite the
documentation of libxl_xen_console_read_line(), line_r is not
nul-terminated if 16384 characters were copied to the buffer.

Fix this by asking xc_readconsolering() to fill the buffer up to size
- 1.  As the number of characters in the buffer is only needed in
libxl_xen_console_read_line(), make it a local variable there instead
of part of the libxl__xen_console_reader struct.

Fixes: 4024bae739cc ("xl: Add subcommand 'xl dmesg'")
Reported-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: bb03169bcb6ecccf372de1f6b9285cd519a26bb8
master date: 2024-09-03 10:53:44 +0100

diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
index a563c9d3c7f9eda57e79b15efd302f06b3714fb8..9f736b891335035113d0272f6042102b5fa761ba 100644 (file)
--- a/tools/libs/light/libxl_console.c
+++ b/tools/libs/light/libxl_console.c
@@ -774,12 +774,17 @@ libxl_xen_console_reader *
 {
     GC_INIT(ctx);
     libxl_xen_console_reader *cr;
-    unsigned int size = 16384;
+    /*
+     * We want xen to fill the buffer in as few hypercalls as
+     * possible, but xen will not nul-terminate it.  The default size
+     * of Xen's console buffer is 16384.  Leave one byte at the end
+     * for the null character.
+     */
+    unsigned int size = 16384 + 1;
 
     cr = libxl__zalloc(NOGC, sizeof(libxl_xen_console_reader));
     cr->buffer = libxl__zalloc(NOGC, size);
     cr->size = size;
-    cr->count = size;
     cr->clear = clear;
     cr->incremental = 1;
 
@@ -800,10 +805,16 @@ int libxl_xen_console_read_line(libxl_ctx *ctx,
                                 char **line_r)
 {
     int ret;
+    /*
+     * Number of chars to copy into the buffer.  xc_readconsolering()
+     * does not add a null character at the end, so leave a space for
+     * us to add it.
+     */
+    unsigned int nr_chars = cr->size - 1;
     GC_INIT(ctx);
 
     memset(cr->buffer, 0, cr->size);
-    ret = xc_readconsolering(ctx->xch, cr->buffer, &cr->count,
+    ret = xc_readconsolering(ctx->xch, cr->buffer, &nr_chars,
                              cr->clear, cr->incremental, &cr->index);
     if (ret < 0) {
         LOGE(ERROR, "reading console ring buffer");
@@ -811,7 +822,7 @@ int libxl_xen_console_read_line(libxl_ctx *ctx,
         return ERROR_FAIL;
     }
     if (!ret) {
-        if (cr->count) {
+        if (nr_chars) {
             *line_r = cr->buffer;
             ret = 1;
         } else {

diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index d5732d1c37920bb34b0883c1cef2c4f9ffaf6838..e5477c7668c118ab6e776d44108e5bc138762fa8 100644 (file)
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -2080,7 +2080,6 @@ _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 struct libxl__xen_console_reader {
     char *buffer;
     unsigned int size;
-    unsigned int count;
     unsigned int clear;
     unsigned int incremental;
     unsigned int index;