From: Mathieu Desnoyers Date: Mon, 5 Sep 2022 22:19:16 +0000 (-0400) Subject: Fix: capture_sequence_element_{un,}signed: handle user-space input X-Git-Tag: v2.13.6~9 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=18789896935a1a16a5be8ff6fd6279af206af27c;p=lttng-modules.git Fix: capture_sequence_element_{un,}signed: handle user-space input 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 Change-Id: Icf0c141d333f63402d8a76051bcd53fcdd5ed8c2 --- diff --git a/src/lttng-event-notifier-notification.c b/src/lttng-event-notifier-notification.c index 054d3339..ae35b1f4 100644 --- a/src/lttng-event-notifier-notification.c +++ b/src/lttng-event-notifier-notification.c @@ -12,6 +12,7 @@ #include #include #include +#include #include /* @@ -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; }