Fix: capture_sequence_element_{un,}signed: handle user-space input
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Mon, 5 Sep 2022 22:19:16 +0000 (18:19 -0400)
committerMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Wed, 7 Sep 2022 20:07:38 +0000 (16:07 -0400)
The "user" attribute (copy from userspace) is not applied to
sequence/array of integer field capture within event notifications. This
could eventually lead to unsafe copy of integers from user-space.

Currently, the only array/sequence of integers which are read from
user-space are the arguments to sys_select (e.g. `readfds` field). Those
are expressed as "custom" fields, which are skipped by the filter and
capture bytecode.

This is therefore not an issue with the current instrumentation, but we
should properly handle this nevertheless.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Icf0c141d333f63402d8a76051bcd53fcdd5ed8c2

src/lttng-event-notifier-notification.c

index 054d33393f459832fac3906e81ba634cfe8c41f9..ae35b1f45697e78d774f067d2fe665dd23e8bc2d 100644 (file)
@@ -12,6 +12,7 @@
 #include <lttng/msgpack.h>
 #include <lttng/event-notifier-notification.h>
 #include <lttng/events-internal.h>
+#include <lttng/probe-user.h>
 #include <wrapper/barrier.h>
 
 /*
@@ -100,39 +101,65 @@ int64_t capture_sequence_element_signed(uint8_t *ptr,
 {
        int64_t value = 0;
        unsigned int size = type->size;
+       bool user = type->user;
        bool byte_order_reversed = type->reverse_byte_order;
 
        switch (size) {
        case 8:
-               value = *ptr;
+       {
+               int8_t tmp;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(int8_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *ptr;
+               }
+               value = tmp;
                break;
+       }
        case 16:
        {
                int16_t tmp;
-               tmp = *(int16_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(int16_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(int16_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab16s(&tmp);
-
                value = tmp;
                break;
        }
        case 32:
        {
                int32_t tmp;
-               tmp = *(int32_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(int32_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(int32_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab32s(&tmp);
-
                value = tmp;
                break;
        }
        case 64:
        {
                int64_t tmp;
-               tmp = *(int64_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(int64_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(int64_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab64s(&tmp);
-
                value = tmp;
                break;
        }
@@ -149,39 +176,65 @@ uint64_t capture_sequence_element_unsigned(uint8_t *ptr,
 {
        uint64_t value = 0;
        unsigned int size = type->size;
+       bool user = type->user;
        bool byte_order_reversed = type->reverse_byte_order;
 
        switch (size) {
        case 8:
-               value = *ptr;
+       {
+               uint8_t tmp;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(uint8_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *ptr;
+               }
+               value = tmp;
                break;
+       }
        case 16:
        {
                uint16_t tmp;
-               tmp = *(uint16_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(uint16_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(uint16_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab16s(&tmp);
-
                value = tmp;
                break;
        }
        case 32:
        {
                uint32_t tmp;
-               tmp = *(uint32_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(uint32_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(uint32_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab32s(&tmp);
-
                value = tmp;
                break;
        }
        case 64:
        {
                uint64_t tmp;
-               tmp = *(uint64_t *) ptr;
+
+               if (user) {
+                       if (lttng_copy_from_user_check_nofault(&tmp, ptr, sizeof(uint64_t)))
+                               tmp = 0;
+               } else {
+                       tmp = *(uint64_t *) ptr;
+               }
                if (byte_order_reversed)
                        __swab64s(&tmp);
-
                value = tmp;
                break;
        }
This page took 0.027617 seconds and 4 git commands to generate.