From c89271d467715e782b76a87c8e26859ec04d9aff Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 29 Sep 2022 15:54:41 -0400 Subject: [PATCH] Fix: bytecode interpreter: LOAD_FIELD: handle user fields The instructions for recursive traversal through composed types are used by the filter expressions which access fields nested within composed types. Instructions BYTECODE_OP_LOAD_FIELD_STRING and BYTECODE_OP_LOAD_FIELD_SEQUENCE were leaving the "user" attribute uninitialized. Initialize those to 0. The handling of userspace strings and integers is missing in LOAD_FIELD instructions. Therefore, ensure that the specialization leaves the generic LOAD_FIELD instruction in place for userspace input. Add a "user" attribute to: - struct bytecode_get_index_data elem field (produced by the specialization), - struct vstack_load used by the specialization, - struct load_ptr used by the interpreter. Use this "user" attribute in dynamic_load_field() for integer, string and string_sequence object types to ensure that the proper userspace-aware accesses are performed when loading those fields. This prevents events with userspace input arguments (e.g. pipe2 system call fildes field) from oopsing the kernel or reading arbitrary kernel memory when used by the filter bytecode. Signed-off-by: Mathieu Desnoyers Change-Id: Id9c373ff1a70e162ba913e5592437249a4947c96 --- lttng-filter-interpreter.c | 122 +++++++++++++++++++++++++++++-------- lttng-filter-specialize.c | 46 ++++++++------ lttng-filter.h | 5 +- 3 files changed, 129 insertions(+), 44 deletions(-) diff --git a/lttng-filter-interpreter.c b/lttng-filter-interpreter.c index 8c870e14..a9af74ad 100644 --- a/lttng-filter-interpreter.c +++ b/lttng-filter-interpreter.c @@ -15,6 +15,7 @@ #include #include +#include LTTNG_STACK_FRAME_NON_STANDARD(lttng_filter_interpret_bytecode); @@ -291,6 +292,7 @@ static int context_get_index(struct lttng_probe_ctx *lttng_probe_ctx, ptr->ptr = &ptr->u.u64; } ptr->rev_bo = field->type.u.basic.integer.reverse_byte_order; + ptr->user = field->type.u.basic.integer.user; break; case atype_enum: { @@ -308,6 +310,7 @@ static int context_get_index(struct lttng_probe_ctx *lttng_probe_ctx, ptr->ptr = &ptr->u.u64; } ptr->rev_bo = itype->reverse_byte_order; + ptr->user = itype->user; break; } case atype_array: @@ -322,6 +325,7 @@ static int context_get_index(struct lttng_probe_ctx *lttng_probe_ctx, ptr->object_type = OBJECT_TYPE_STRING; ctx_field->get_value(ctx_field, lttng_probe_ctx, &v); ptr->ptr = v.str; + ptr->user = field->type.u.array.elem_type.u.basic.integer.user; break; case atype_sequence: if (field->type.u.sequence.elem_type.atype != atype_integer) { @@ -335,6 +339,7 @@ static int context_get_index(struct lttng_probe_ctx *lttng_probe_ctx, ptr->object_type = OBJECT_TYPE_STRING; ctx_field->get_value(ctx_field, lttng_probe_ctx, &v); ptr->ptr = v.str; + ptr->user = field->type.u.sequence.elem_type.u.basic.integer.user; break; case atype_array_bitfield: printk(KERN_WARNING "Bitfield array type is not supported.\n"); @@ -346,6 +351,7 @@ static int context_get_index(struct lttng_probe_ctx *lttng_probe_ctx, ptr->object_type = OBJECT_TYPE_STRING; ctx_field->get_value(ctx_field, lttng_probe_ctx, &v); ptr->ptr = v.str; + ptr->user = field->type.u.basic.string.user; break; case atype_struct: printk(KERN_WARNING "Structure type cannot be loaded.\n"); @@ -386,6 +392,7 @@ static int dynamic_get_index(struct lttng_probe_ctx *lttng_probe_ctx, stack_top->u.ptr.ptr = ptr; stack_top->u.ptr.object_type = gid->elem.type; stack_top->u.ptr.rev_bo = gid->elem.rev_bo; + stack_top->u.ptr.user = gid->elem.user; /* field is only used for types nested within variants. */ stack_top->u.ptr.field = NULL; break; @@ -405,6 +412,7 @@ static int dynamic_get_index(struct lttng_probe_ctx *lttng_probe_ctx, stack_top->u.ptr.ptr = ptr; stack_top->u.ptr.object_type = gid->elem.type; stack_top->u.ptr.rev_bo = gid->elem.rev_bo; + stack_top->u.ptr.user = gid->elem.user; /* field is only used for types nested within variants. */ stack_top->u.ptr.field = NULL; break; @@ -442,6 +450,7 @@ static int dynamic_get_index(struct lttng_probe_ctx *lttng_probe_ctx, /* field is only used for types nested within variants. */ stack_top->u.ptr.field = NULL; stack_top->u.ptr.rev_bo = gid->elem.rev_bo; + stack_top->u.ptr.user = gid->elem.user; break; } return 0; @@ -468,14 +477,24 @@ static int dynamic_load_field(struct estack_entry *stack_top) switch (stack_top->u.ptr.object_type) { case OBJECT_TYPE_S8: dbg_printk("op load field s8\n"); - stack_top->u.v = *(int8_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&stack_top->u.v, (int8_t __user *) stack_top->u.ptr.ptr, sizeof(int8_t))) + stack_top->u.v = 0; + } else { + stack_top->u.v = *(int8_t *) stack_top->u.ptr.ptr; + } break; case OBJECT_TYPE_S16: { int16_t tmp; dbg_printk("op load field s16\n"); - tmp = *(int16_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (int16_t __user *) stack_top->u.ptr.ptr, sizeof(int16_t))) + tmp = 0; + } else { + tmp = *(int16_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab16s(&tmp); stack_top->u.v = tmp; @@ -486,7 +505,12 @@ static int dynamic_load_field(struct estack_entry *stack_top) int32_t tmp; dbg_printk("op load field s32\n"); - tmp = *(int32_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (int32_t __user *) stack_top->u.ptr.ptr, sizeof(int32_t))) + tmp = 0; + } else { + tmp = *(int32_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab32s(&tmp); stack_top->u.v = tmp; @@ -497,7 +521,12 @@ static int dynamic_load_field(struct estack_entry *stack_top) int64_t tmp; dbg_printk("op load field s64\n"); - tmp = *(int64_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (int64_t __user *) stack_top->u.ptr.ptr, sizeof(int64_t))) + tmp = 0; + } else { + tmp = *(int64_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab64s(&tmp); stack_top->u.v = tmp; @@ -505,14 +534,24 @@ static int dynamic_load_field(struct estack_entry *stack_top) } case OBJECT_TYPE_U8: dbg_printk("op load field u8\n"); - stack_top->u.v = *(uint8_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&stack_top->u.v, (uint8_t __user *) stack_top->u.ptr.ptr, sizeof(uint8_t))) + stack_top->u.v = 0; + } else { + stack_top->u.v = *(uint8_t *) stack_top->u.ptr.ptr; + } break; case OBJECT_TYPE_U16: { uint16_t tmp; dbg_printk("op load field s16\n"); - tmp = *(uint16_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (uint16_t __user *) stack_top->u.ptr.ptr, sizeof(uint16_t))) + tmp = 0; + } else { + tmp = *(uint16_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab16s(&tmp); stack_top->u.v = tmp; @@ -523,7 +562,12 @@ static int dynamic_load_field(struct estack_entry *stack_top) uint32_t tmp; dbg_printk("op load field u32\n"); - tmp = *(uint32_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (uint32_t __user *) stack_top->u.ptr.ptr, sizeof(uint32_t))) + tmp = 0; + } else { + tmp = *(uint32_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab32s(&tmp); stack_top->u.v = tmp; @@ -534,7 +578,12 @@ static int dynamic_load_field(struct estack_entry *stack_top) uint64_t tmp; dbg_printk("op load field u64\n"); - tmp = *(uint64_t *) stack_top->u.ptr.ptr; + if (stack_top->u.ptr.user) { + if (lttng_copy_from_user_check_nofault(&tmp, (uint64_t __user *) stack_top->u.ptr.ptr, sizeof(uint64_t))) + tmp = 0; + } else { + tmp = *(uint64_t *) stack_top->u.ptr.ptr; + } if (stack_top->u.ptr.rev_bo) __swab64s(&tmp); stack_top->u.v = tmp; @@ -542,19 +591,30 @@ static int dynamic_load_field(struct estack_entry *stack_top) } case OBJECT_TYPE_STRING: { - const char *str; + dbg_printk("op load field string: user=%d\n", stack_top->u.ptr.user); + if (stack_top->u.ptr.user) { + const char __user *user_str = (const char __user *) stack_top->u.ptr.ptr; - dbg_printk("op load field string\n"); - str = (const char *) stack_top->u.ptr.ptr; - stack_top->u.s.str = str; - if (unlikely(!stack_top->u.s.str)) { - dbg_printk("Filter warning: loading a NULL string.\n"); - ret = -EINVAL; - goto end; + stack_top->u.s.user_str = user_str; + if (unlikely(!stack_top->u.s.user_str)) { + dbg_printk("Bytecode warning: loading a NULL user string.\n"); + ret = -EINVAL; + goto end; + } + stack_top->u.s.user = 1; + } else { + const char *str = (const char *) stack_top->u.ptr.ptr; + + stack_top->u.s.str = str; + if (unlikely(!stack_top->u.s.str)) { + dbg_printk("Bytecode warning: loading a NULL string.\n"); + ret = -EINVAL; + goto end; + } + stack_top->u.s.user = 0; } stack_top->u.s.seq_len = LTTNG_SIZE_MAX; - stack_top->u.s.literal_type = - ESTACK_STRING_LITERAL_TYPE_NONE; + stack_top->u.s.literal_type = ESTACK_STRING_LITERAL_TYPE_NONE; break; } case OBJECT_TYPE_STRING_SEQUENCE: @@ -564,14 +624,24 @@ static int dynamic_load_field(struct estack_entry *stack_top) dbg_printk("op load field string sequence\n"); ptr = stack_top->u.ptr.ptr; stack_top->u.s.seq_len = *(unsigned long *) ptr; - stack_top->u.s.str = *(const char **) (ptr + sizeof(unsigned long)); - if (unlikely(!stack_top->u.s.str)) { - dbg_printk("Filter warning: loading a NULL sequence.\n"); - ret = -EINVAL; - goto end; + if (stack_top->u.ptr.user) { + stack_top->u.s.user_str = *(const char __user **) (ptr + sizeof(unsigned long)); + if (unlikely(!stack_top->u.s.user_str)) { + dbg_printk("Bytecode warning: loading a NULL user sequence.\n"); + ret = -EINVAL; + goto end; + } + stack_top->u.s.user = 1; + } else { + stack_top->u.s.str = *(const char **) (ptr + sizeof(unsigned long)); + if (unlikely(!stack_top->u.s.str)) { + dbg_printk("Bytecode warning: loading a NULL sequence.\n"); + ret = -EINVAL; + goto end; + } + stack_top->u.s.user = 0; } - stack_top->u.s.literal_type = - ESTACK_STRING_LITERAL_TYPE_NONE; + stack_top->u.s.literal_type = ESTACK_STRING_LITERAL_TYPE_NONE; break; } case OBJECT_TYPE_DYNAMIC: @@ -1575,6 +1645,7 @@ uint64_t lttng_filter_interpret_bytecode(void *filter_data, estack_ax(stack, top)->u.s.seq_len = LTTNG_SIZE_MAX; estack_ax(stack, top)->u.s.literal_type = ESTACK_STRING_LITERAL_TYPE_NONE; + estack_ax(stack, top)->u.s.user = 0; next_pc += sizeof(struct load_op); PO; } @@ -1594,6 +1665,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 = 0; next_pc += sizeof(struct load_op); PO; } diff --git a/lttng-filter-specialize.c b/lttng-filter-specialize.c index 7780f64a..2f60ee40 100644 --- a/lttng-filter-specialize.c +++ b/lttng-filter-specialize.c @@ -76,48 +76,49 @@ static int specialize_load_field(struct vstack_entry *stack_top, case OBJECT_TYPE_S8: dbg_printk("op load field s8\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_S8; break; case OBJECT_TYPE_S16: dbg_printk("op load field s16\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_S16; break; case OBJECT_TYPE_S32: dbg_printk("op load field s32\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_S32; break; case OBJECT_TYPE_S64: dbg_printk("op load field s64\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_S64; break; case OBJECT_TYPE_U8: dbg_printk("op load field u8\n"); stack_top->type = REG_S64; - insn->op = FILTER_OP_LOAD_FIELD_U8; + if (!stack_top->load.user) + insn->op = FILTER_OP_LOAD_FIELD_U8; break; case OBJECT_TYPE_U16: dbg_printk("op load field u16\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_U16; break; case OBJECT_TYPE_U32: dbg_printk("op load field u32\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_U32; break; case OBJECT_TYPE_U64: dbg_printk("op load field u64\n"); stack_top->type = REG_S64; - if (!stack_top->load.rev_bo) + if (!stack_top->load.rev_bo && !stack_top->load.user) insn->op = FILTER_OP_LOAD_FIELD_U64; break; case OBJECT_TYPE_DOUBLE: @@ -127,12 +128,14 @@ static int specialize_load_field(struct vstack_entry *stack_top, case OBJECT_TYPE_STRING: dbg_printk("op load field string\n"); stack_top->type = REG_STRING; - insn->op = FILTER_OP_LOAD_FIELD_STRING; + if (!stack_top->load.user) + insn->op = FILTER_OP_LOAD_FIELD_STRING; break; case OBJECT_TYPE_STRING_SEQUENCE: dbg_printk("op load field string sequence\n"); stack_top->type = REG_STRING; - insn->op = FILTER_OP_LOAD_FIELD_SEQUENCE; + if (!stack_top->load.user) + insn->op = FILTER_OP_LOAD_FIELD_SEQUENCE; break; case OBJECT_TYPE_DYNAMIC: ret = -EINVAL; @@ -220,9 +223,8 @@ static int specialize_get_index(struct bytecode_runtime *runtime, gid.array_len = num_elems * (elem_len / CHAR_BIT); gid.elem.type = stack_top->load.object_type; gid.elem.len = elem_len; - if (field->type.u.array.elem_type.u.basic.integer.reverse_byte_order) - gid.elem.rev_bo = true; - stack_top->load.rev_bo = gid.elem.rev_bo; + stack_top->load.rev_bo = gid.elem.rev_bo = field->type.u.array.elem_type.u.basic.integer.reverse_byte_order; + stack_top->load.user = gid.elem.user = field->type.u.array.elem_type.u.basic.integer.user; break; } case OBJECT_TYPE_SEQUENCE: @@ -241,9 +243,8 @@ static int specialize_get_index(struct bytecode_runtime *runtime, gid.offset = index * (elem_len / CHAR_BIT); gid.elem.type = stack_top->load.object_type; gid.elem.len = elem_len; - if (field->type.u.sequence.elem_type.u.basic.integer.reverse_byte_order) - gid.elem.rev_bo = true; - stack_top->load.rev_bo = gid.elem.rev_bo; + stack_top->load.rev_bo = gid.elem.rev_bo = field->type.u.sequence.elem_type.u.basic.integer.reverse_byte_order; + stack_top->load.user = gid.elem.user = field->type.u.sequence.elem_type.u.basic.integer.user; break; } case OBJECT_TYPE_STRUCT: @@ -312,7 +313,8 @@ static int specialize_load_object(const struct lttng_event_field *field, load->object_type = OBJECT_TYPE_S64; else load->object_type = OBJECT_TYPE_U64; - load->rev_bo = false; + load->rev_bo = field->type.u.basic.integer.reverse_byte_order; + load->user = field->type.u.basic.integer.user; break; case atype_enum: { @@ -323,7 +325,8 @@ static int specialize_load_object(const struct lttng_event_field *field, load->object_type = OBJECT_TYPE_S64; else load->object_type = OBJECT_TYPE_U64; - load->rev_bo = false; + load->rev_bo = itype->reverse_byte_order; + load->user = itype->user; break; } case atype_array: @@ -333,12 +336,14 @@ static int specialize_load_object(const struct lttng_event_field *field, } if (is_context) { load->object_type = OBJECT_TYPE_STRING; + load->user = field->type.u.array.elem_type.u.basic.integer.user; } else { if (field->type.u.array.elem_type.u.basic.integer.encoding == lttng_encode_none) { load->object_type = OBJECT_TYPE_ARRAY; load->field = field; } else { load->object_type = OBJECT_TYPE_STRING_SEQUENCE; + load->user = field->type.u.array.elem_type.u.basic.integer.user; } } break; @@ -349,12 +354,14 @@ static int specialize_load_object(const struct lttng_event_field *field, } if (is_context) { load->object_type = OBJECT_TYPE_STRING; + load->user = field->type.u.sequence.elem_type.u.basic.integer.user; } else { if (field->type.u.sequence.elem_type.u.basic.integer.encoding == lttng_encode_none) { load->object_type = OBJECT_TYPE_SEQUENCE; load->field = field; } else { load->object_type = OBJECT_TYPE_STRING_SEQUENCE; + load->user = field->type.u.sequence.elem_type.u.basic.integer.user; } } break; @@ -366,6 +373,7 @@ static int specialize_load_object(const struct lttng_event_field *field, return -EINVAL; case atype_string: load->object_type = OBJECT_TYPE_STRING; + load->user = field->type.u.basic.string.user; break; case atype_struct: printk(KERN_WARNING "Structure type cannot be loaded.\n"); @@ -402,6 +410,7 @@ static int specialize_context_lookup(struct bytecode_runtime *runtime, gid.ctx_index = idx; gid.elem.type = load->object_type; gid.elem.rev_bo = load->rev_bo; + gid.elem.user = load->user; data_offset = bytecode_push_data(runtime, &gid, __alignof__(gid), sizeof(gid)); if (data_offset < 0) { @@ -472,6 +481,7 @@ static int specialize_event_payload_lookup(struct lttng_event *event, gid.offset = field_offset; gid.elem.type = load->object_type; gid.elem.rev_bo = load->rev_bo; + gid.elem.user = load->user; data_offset = bytecode_push_data(runtime, &gid, __alignof__(gid), sizeof(gid)); if (data_offset < 0) { diff --git a/lttng-filter.h b/lttng-filter.h index fec2db1f..e82d883d 100644 --- a/lttng-filter.h +++ b/lttng-filter.h @@ -91,6 +91,7 @@ struct filter_get_index_data { size_t len; enum object_type type; bool rev_bo; /* reverse byte order */ + bool user; /* from userspace */ } elem; }; @@ -100,6 +101,7 @@ struct vstack_load { enum object_type object_type; const struct lttng_event_field *field; bool rev_bo; /* reverse byte order */ + bool user; /* from userspace */ }; struct vstack_entry { @@ -168,6 +170,7 @@ struct load_ptr { enum object_type object_type; const void *ptr; bool rev_bo; + bool user; /* from userspace */ /* Temporary place-holders for contexts. */ union { int64_t s64; @@ -190,7 +193,7 @@ struct estack_entry { const char __user *user_str; size_t seq_len; enum estack_string_literal_type literal_type; - int user; /* is string from userspace ? */ + bool user; /* is string from userspace ? */ } s; struct load_ptr ptr; } u; -- 2.34.1