diff options
author | Jason M. Bills <jason.m.bills@linux.intel.com> | 2021-05-24 22:35:24 +0300 |
---|---|---|
committer | Jason M. Bills <jason.m.bills@linux.intel.com> | 2021-05-24 22:35:24 +0300 |
commit | 0e0df451ae365f09d5c0c766b253f23de26901f2 (patch) | |
tree | db4d7d3ce85e02ee01ad58a86ede02ac876aae77 /poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch | |
parent | e370fd750e2821620ec427f26f8efab0069824ff (diff) | |
parent | 7e10dee74964afa47859704886128dd256acf854 (diff) | |
download | openbmc-0e0df451ae365f09d5c0c766b253f23de26901f2.tar.xz |
Merge tag '0.52' of ssh://git-amr-1.devtools.intel.com:29418/openbmc-openbmc into update
Diffstat (limited to 'poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch')
-rw-r--r-- | poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch | 159 |
1 files changed, 159 insertions, 0 deletions
diff --git a/poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch b/poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch new file mode 100644 index 000000000..609690f05 --- /dev/null +++ b/poky/meta/recipes-kernel/lttng/lttng-modules/0002-Fix-filter-interpreter-early-exits-on-uninitialized-.patch @@ -0,0 +1,159 @@ +From 23a2f61ffc6a656f136fa2044c0c3b8f79766779 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Galarneau?= + <jeremie.galarneau@efficios.com> +Date: Wed, 3 Mar 2021 18:52:19 -0500 +Subject: [PATCH 2/4] Fix: filter interpreter early-exits on uninitialized + value +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +I observed that syscall filtering on string arguments wouldn't work on +my development machines, both running 5.11.2-arch1-1 (Arch Linux). + +For instance, enabling the tracing of the `openat()` syscall with the +'filename == "/proc/cpuinfo"' filter would not produce events even +though matching events were present in another session that had no +filtering active. The same problem occurred with `execve()`. + +I tried a couple of kernel versions before (5.11.1 and 5.10.13, if +memory serves me well) and I had the same problem. Meanwhile, I couldn't +reproduce the problem on various Debian machines (the LTTng CI) nor on a +fresh Ubuntu 20.04 with both the stock kernel and with an updated 5.11.2 +kernel. + +I built the lttng-modules with the interpreter debugging printout and +saw the following warning: + LTTng: [debug bytecode in /home/jgalar/EfficiOS/src/lttng-modules/src/lttng-bytecode-interpreter.c:bytecode_interpret@1508] Bytecode warning: loading a NULL string. + +After a shedload (yes, a _shed_load) of digging, I figured that the +problem was hidden in plain sight near that logging statement. + +In the `BYTECODE_OP_LOAD_FIELD_REF_USER_STRING` operation, the 'ax' +register's 'user_str' is initialized with the stack value (the user +space string's address in our case). However, a NULL check is performed +against the register's 'str' member. + +I initialy suspected that both members would be part of the same union +and alias each-other, but they are actually contiguous in a structure. + +On the unaffected machines, I could confirm that the `str` member was +uninitialized to a non-zero value causing the condition to evaluate to +false. + +Francis Deslauriers reproduced the problem by initializing the +interpreter stack to zero. + +I am unsure of the exact kernel configuration option that reveals this +issue on Arch Linux, but my kernel has the following option enabled: + +CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL: + Zero-initialize any stack variables that may be passed by reference + and had not already been explicitly initialized. This is intended to + eliminate all classes of uninitialized stack variable exploits and + information exposures. + +I have not tried to build without this enabled as, anyhow, this seems +to be a legitimate issue. + +I have spotted what appears to be an identical problem in +`BYTECODE_OP_LOAD_FIELD_REF_USER_SEQUENCE` and corrected it. However, +I have not exercised that code path. + +The commit that introduced this problem is 5b4ad89. + +The debug print-out of the `BYTECODE_OP_LOAD_FIELD_REF_USER_STRING` +operation is modified to print the user string (truncated to 31 chars). + +Upstream-status: backport + +Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> +Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> +Change-Id: I2da3c31b9e3ce0e1b164cf3d2711c0893cbec273 +--- + lttng-filter-interpreter.c | 41 ++++++++++++++++++++++++++++++++++---- + 1 file changed, 37 insertions(+), 4 deletions(-) + +diff --git a/lttng-filter-interpreter.c b/lttng-filter-interpreter.c +index 5d572437..6e5a5139 100644 +--- a/lttng-filter-interpreter.c ++++ b/lttng-filter-interpreter.c +@@ -22,7 +22,7 @@ LTTNG_STACK_FRAME_NON_STANDARD(lttng_filter_interpret_bytecode); + * to handle user-space read. + */ + static +-char get_char(struct estack_entry *reg, size_t offset) ++char get_char(const struct estack_entry *reg, size_t offset) + { + if (unlikely(offset >= reg->u.s.seq_len)) + return '\0'; +@@ -593,6 +593,39 @@ end: + return ret; + } + ++#ifdef DEBUG ++ ++#define DBG_USER_STR_CUTOFF 32 ++ ++/* ++ * In debug mode, print user string (truncated, if necessary). ++ */ ++static inline ++void dbg_load_ref_user_str_printk(const struct estack_entry *user_str_reg) ++{ ++ size_t pos = 0; ++ char last_char; ++ char user_str[DBG_USER_STR_CUTOFF]; ++ ++ pagefault_disable(); ++ do { ++ last_char = get_char(user_str_reg, pos); ++ user_str[pos] = last_char; ++ pos++; ++ } while (last_char != '\0' && pos < sizeof(user_str)); ++ pagefault_enable(); ++ ++ user_str[sizeof(user_str) - 1] = '\0'; ++ dbg_printk("load field ref user string: '%s%s'\n", user_str, ++ last_char != '\0' ? "[...]" : ""); ++} ++#else ++static inline ++void dbg_load_ref_user_str_printk(const struct estack_entry *user_str_reg) ++{ ++} ++#endif ++ + /* + * Return 0 (discard), or raise the 0x1 flag (log event). + * Currently, other flags are kept for future extensions and have no +@@ -1313,7 +1346,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, + estack_push(stack, top, ax, bx); + estack_ax(stack, top)->u.s.user_str = + *(const char * const *) &filter_stack_data[ref->offset]; +- if (unlikely(!estack_ax(stack, top)->u.s.str)) { ++ if (unlikely(!estack_ax(stack, top)->u.s.user_str)) { + dbg_printk("Filter warning: loading a NULL string.\n"); + ret = -EINVAL; + goto end; +@@ -1322,7 +1355,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, + estack_ax(stack, top)->u.s.literal_type = + ESTACK_STRING_LITERAL_TYPE_NONE; + estack_ax(stack, top)->u.s.user = 1; +- dbg_printk("ref load string %s\n", estack_ax(stack, top)->u.s.str); ++ dbg_load_ref_user_str_printk(estack_ax(stack, top)); + next_pc += sizeof(struct load_op) + sizeof(struct field_ref); + PO; + } +@@ -1340,7 +1373,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, + estack_ax(stack, top)->u.s.user_str = + *(const char **) (&filter_stack_data[ref->offset + + sizeof(unsigned long)]); +- if (unlikely(!estack_ax(stack, top)->u.s.str)) { ++ if (unlikely(!estack_ax(stack, top)->u.s.user_str)) { + dbg_printk("Filter warning: loading a NULL sequence.\n"); + ret = -EINVAL; + goto end; +-- +2.19.1 + |