1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
|
From c18635fd69fc2da238f00a26ab707f1b2a50bf64 Mon Sep 17 00:00:00 2001
From: Javi Merino <javi.merino@cloud.com>
Date: Tue, 24 Sep 2024 14:41:06 +0200
Subject: [PATCH 28/35] libxl: Fix nul-termination of the return value of
libxl_xen_console_read_line()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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
---
tools/libs/light/libxl_console.c | 19 +++++++++++++++----
tools/libs/light/libxl_internal.h | 1 -
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
index a563c9d3c7..9f736b8913 100644
--- 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 3b58bb2d7f..96d14f5746 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -2077,7 +2077,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;
--
2.46.1
|