Andrew Geissler | 95ac1b8 | 2021-03-31 14:34:31 -0500 | [diff] [blame^] | 1 | From 23a2f61ffc6a656f136fa2044c0c3b8f79766779 Mon Sep 17 00:00:00 2001 |
| 2 | From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Galarneau?= |
| 3 | <jeremie.galarneau@efficios.com> |
| 4 | Date: Wed, 3 Mar 2021 18:52:19 -0500 |
| 5 | Subject: [PATCH 2/4] Fix: filter interpreter early-exits on uninitialized |
| 6 | value |
| 7 | MIME-Version: 1.0 |
| 8 | Content-Type: text/plain; charset=UTF-8 |
| 9 | Content-Transfer-Encoding: 8bit |
| 10 | |
| 11 | I observed that syscall filtering on string arguments wouldn't work on |
| 12 | my development machines, both running 5.11.2-arch1-1 (Arch Linux). |
| 13 | |
| 14 | For instance, enabling the tracing of the `openat()` syscall with the |
| 15 | 'filename == "/proc/cpuinfo"' filter would not produce events even |
| 16 | though matching events were present in another session that had no |
| 17 | filtering active. The same problem occurred with `execve()`. |
| 18 | |
| 19 | I tried a couple of kernel versions before (5.11.1 and 5.10.13, if |
| 20 | memory serves me well) and I had the same problem. Meanwhile, I couldn't |
| 21 | reproduce the problem on various Debian machines (the LTTng CI) nor on a |
| 22 | fresh Ubuntu 20.04 with both the stock kernel and with an updated 5.11.2 |
| 23 | kernel. |
| 24 | |
| 25 | I built the lttng-modules with the interpreter debugging printout and |
| 26 | saw the following warning: |
| 27 | LTTng: [debug bytecode in /home/jgalar/EfficiOS/src/lttng-modules/src/lttng-bytecode-interpreter.c:bytecode_interpret@1508] Bytecode warning: loading a NULL string. |
| 28 | |
| 29 | After a shedload (yes, a _shed_load) of digging, I figured that the |
| 30 | problem was hidden in plain sight near that logging statement. |
| 31 | |
| 32 | In the `BYTECODE_OP_LOAD_FIELD_REF_USER_STRING` operation, the 'ax' |
| 33 | register's 'user_str' is initialized with the stack value (the user |
| 34 | space string's address in our case). However, a NULL check is performed |
| 35 | against the register's 'str' member. |
| 36 | |
| 37 | I initialy suspected that both members would be part of the same union |
| 38 | and alias each-other, but they are actually contiguous in a structure. |
| 39 | |
| 40 | On the unaffected machines, I could confirm that the `str` member was |
| 41 | uninitialized to a non-zero value causing the condition to evaluate to |
| 42 | false. |
| 43 | |
| 44 | Francis Deslauriers reproduced the problem by initializing the |
| 45 | interpreter stack to zero. |
| 46 | |
| 47 | I am unsure of the exact kernel configuration option that reveals this |
| 48 | issue on Arch Linux, but my kernel has the following option enabled: |
| 49 | |
| 50 | CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL: |
| 51 | Zero-initialize any stack variables that may be passed by reference |
| 52 | and had not already been explicitly initialized. This is intended to |
| 53 | eliminate all classes of uninitialized stack variable exploits and |
| 54 | information exposures. |
| 55 | |
| 56 | I have not tried to build without this enabled as, anyhow, this seems |
| 57 | to be a legitimate issue. |
| 58 | |
| 59 | I have spotted what appears to be an identical problem in |
| 60 | `BYTECODE_OP_LOAD_FIELD_REF_USER_SEQUENCE` and corrected it. However, |
| 61 | I have not exercised that code path. |
| 62 | |
| 63 | The commit that introduced this problem is 5b4ad89. |
| 64 | |
| 65 | The debug print-out of the `BYTECODE_OP_LOAD_FIELD_REF_USER_STRING` |
| 66 | operation is modified to print the user string (truncated to 31 chars). |
| 67 | |
| 68 | Upstream-status: backport |
| 69 | |
| 70 | Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> |
| 71 | Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
| 72 | Change-Id: I2da3c31b9e3ce0e1b164cf3d2711c0893cbec273 |
| 73 | --- |
| 74 | lttng-filter-interpreter.c | 41 ++++++++++++++++++++++++++++++++++---- |
| 75 | 1 file changed, 37 insertions(+), 4 deletions(-) |
| 76 | |
| 77 | diff --git a/lttng-filter-interpreter.c b/lttng-filter-interpreter.c |
| 78 | index 5d572437..6e5a5139 100644 |
| 79 | --- a/lttng-filter-interpreter.c |
| 80 | +++ b/lttng-filter-interpreter.c |
| 81 | @@ -22,7 +22,7 @@ LTTNG_STACK_FRAME_NON_STANDARD(lttng_filter_interpret_bytecode); |
| 82 | * to handle user-space read. |
| 83 | */ |
| 84 | static |
| 85 | -char get_char(struct estack_entry *reg, size_t offset) |
| 86 | +char get_char(const struct estack_entry *reg, size_t offset) |
| 87 | { |
| 88 | if (unlikely(offset >= reg->u.s.seq_len)) |
| 89 | return '\0'; |
| 90 | @@ -593,6 +593,39 @@ end: |
| 91 | return ret; |
| 92 | } |
| 93 | |
| 94 | +#ifdef DEBUG |
| 95 | + |
| 96 | +#define DBG_USER_STR_CUTOFF 32 |
| 97 | + |
| 98 | +/* |
| 99 | + * In debug mode, print user string (truncated, if necessary). |
| 100 | + */ |
| 101 | +static inline |
| 102 | +void dbg_load_ref_user_str_printk(const struct estack_entry *user_str_reg) |
| 103 | +{ |
| 104 | + size_t pos = 0; |
| 105 | + char last_char; |
| 106 | + char user_str[DBG_USER_STR_CUTOFF]; |
| 107 | + |
| 108 | + pagefault_disable(); |
| 109 | + do { |
| 110 | + last_char = get_char(user_str_reg, pos); |
| 111 | + user_str[pos] = last_char; |
| 112 | + pos++; |
| 113 | + } while (last_char != '\0' && pos < sizeof(user_str)); |
| 114 | + pagefault_enable(); |
| 115 | + |
| 116 | + user_str[sizeof(user_str) - 1] = '\0'; |
| 117 | + dbg_printk("load field ref user string: '%s%s'\n", user_str, |
| 118 | + last_char != '\0' ? "[...]" : ""); |
| 119 | +} |
| 120 | +#else |
| 121 | +static inline |
| 122 | +void dbg_load_ref_user_str_printk(const struct estack_entry *user_str_reg) |
| 123 | +{ |
| 124 | +} |
| 125 | +#endif |
| 126 | + |
| 127 | /* |
| 128 | * Return 0 (discard), or raise the 0x1 flag (log event). |
| 129 | * Currently, other flags are kept for future extensions and have no |
| 130 | @@ -1313,7 +1346,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, |
| 131 | estack_push(stack, top, ax, bx); |
| 132 | estack_ax(stack, top)->u.s.user_str = |
| 133 | *(const char * const *) &filter_stack_data[ref->offset]; |
| 134 | - if (unlikely(!estack_ax(stack, top)->u.s.str)) { |
| 135 | + if (unlikely(!estack_ax(stack, top)->u.s.user_str)) { |
| 136 | dbg_printk("Filter warning: loading a NULL string.\n"); |
| 137 | ret = -EINVAL; |
| 138 | goto end; |
| 139 | @@ -1322,7 +1355,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, |
| 140 | estack_ax(stack, top)->u.s.literal_type = |
| 141 | ESTACK_STRING_LITERAL_TYPE_NONE; |
| 142 | estack_ax(stack, top)->u.s.user = 1; |
| 143 | - dbg_printk("ref load string %s\n", estack_ax(stack, top)->u.s.str); |
| 144 | + dbg_load_ref_user_str_printk(estack_ax(stack, top)); |
| 145 | next_pc += sizeof(struct load_op) + sizeof(struct field_ref); |
| 146 | PO; |
| 147 | } |
| 148 | @@ -1340,7 +1373,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, |
| 149 | estack_ax(stack, top)->u.s.user_str = |
| 150 | *(const char **) (&filter_stack_data[ref->offset |
| 151 | + sizeof(unsigned long)]); |
| 152 | - if (unlikely(!estack_ax(stack, top)->u.s.str)) { |
| 153 | + if (unlikely(!estack_ax(stack, top)->u.s.user_str)) { |
| 154 | dbg_printk("Filter warning: loading a NULL sequence.\n"); |
| 155 | ret = -EINVAL; |
| 156 | goto end; |
| 157 | -- |
| 158 | 2.19.1 |
| 159 | |