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 <mathieu.desnoyers@efficios.com>
Change-Id: Id9c373ff1a70e162ba913e5592437249a4947c96

diff --git a/lttng-filter-interpreter.c b/lttng-filter-interpreter.c
index 8c870e14723db99d6d7e341cbf0e25fb776bff75..a9af74ad0a2c2b8697693c6a91d40e0ef97c0c85 100644 (file)
--- a/lttng-filter-interpreter.c
+++ b/lttng-filter-interpreter.c
@@ -15,6 +15,7 @@
 
 #include <lttng-filter.h>
 #include <lttng-string-utils.h>
+#include <probes/lttng-probe-user.h>
 
 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 7780f64ad3fe1e7fa8f846ae327c1220dd882a14..2f60ee40fe5b118f88bd57ffe0b6260e1a42898a 100644 (file)
--- 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 fec2db1f6c3b2fad866a536587bed97fd959b0dd..e82d883decf0cd1c119e37181b196c678478c768 100644 (file)
--- 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;