From e3091d6a5ecbef6bc823c9e1e9e0da246c9c054c Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Fri, 16 Nov 2018 22:51:06 -0500 Subject: [PATCH] Prevent channel buffer allocation larger than memory Background ========== Until recently (before lttng-modules commit 1f0ab1e) it was possible to trigger an Out-Of-Memory crash by creating a kernel channel buffer larger than the currently usable memory on the system. The following commands was triggering the issue on my laptop: lttng create lttng enable-channel -k --subbuf-size=100G --num-subbuf=1 chan0 The lttng-modules commit 1f0ab1e adds a verification based on an estimate to prevent this from happening. Since this kernel tracer sanity check is based on an estimate, it would safer to do a similar check on the session daemon side. Approach ======== Verify that there is enough memory available on the system to do all the allocations needed to enable the channel. If the available memory is insufficient for the buffer allocation, return an error to the user without trying to allocate the buffers. Use the `/proc/meminfo` procfile to get an estimate of the current size of available memory (using `MemAvailable`). The `MemAvailable` field was added in the Linux kernel 3.14, so if it's absent, fallback to verifying that the requested buffer is smaller than the physical memory on the system. Compute the size of the requested buffers using the following equation: requested_memory = number_subbuffer * size_subbuffer * number_cpu The following error is returned to the command line user: lttng enable-channel -k --subbuf-size=100G --num-subbuf=1 chan0 Error: Channel chan0: Not enough memory (session auto-20181121-161146) Side effect =========== This patch has the interesting side effect to alerting the user with an error that buffer allocation has failed because of memory availability in both --kernel and --userspace channel creation. Drawback ======== The fallback check on older kernels is imperfect and is only to prevent obvious user errors. Note ==== In the future, there might be a need for a way to deactivate this check (by using an environment variable) if a case arises where `/proc/meminfo` doesn't accurately reflect the state of memory for a particular use case. Signed-off-by: Francis Deslauriers --- src/bin/lttng/conf.c | 1 + src/common/utils.c | 88 ++++++++++++++++++++++++++++ src/common/utils.h | 2 + src/lib/lttng-ctl/lttng-ctl.c | 55 +++++++++++++++++ tests/regression/kernel/test_channel | 74 +++++++++++++++++++++++ tests/root_regression | 1 + 6 files changed, 221 insertions(+) create mode 100755 tests/regression/kernel/test_channel diff --git a/src/bin/lttng/conf.c b/src/bin/lttng/conf.c index 364977492..6fc3e6051 100644 --- a/src/bin/lttng/conf.c +++ b/src/bin/lttng/conf.c @@ -183,6 +183,7 @@ int _config_read_session_name(char *path, char **name) int ret = 0; FILE *fp; char var[NAME_MAX], *session_name; + #if (NAME_MAX == 255) #define NAME_MAX_SCANF_IS_A_BROKEN_API "254" #endif diff --git a/src/common/utils.c b/src/common/utils.c index a092d940f..4c000e9b4 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -43,6 +44,19 @@ #include "defaults.h" #include "time.h" +#define PROC_MEMINFO_PATH "/proc/meminfo" +#define PROC_MEMINFO_MEMAVAILABLE_LINE "MemAvailable:" +#define PROC_MEMINFO_MEMTOTAL_LINE "MemTotal:" + +/* The length of the longest field of `/proc/meminfo`. */ +#define PROC_MEMINFO_FIELD_MAX_NAME_LEN 20 + +#if (PROC_MEMINFO_FIELD_MAX_NAME_LEN == 20) +#define MAX_NAME_LEN_SCANF_IS_A_BROKEN_API "19" +#else +#error MAX_NAME_LEN_SCANF_IS_A_BROKEN_API must be updated to match (PROC_MEMINFO_FIELD_MAX_NAME_LEN - 1) +#endif + /* * Return a partial realpath(3) of the path even if the full path does not * exist. For instance, with /tmp/test1/test2/test3, if test2/ does not exist @@ -1673,3 +1687,77 @@ int utils_show_help(int section, const char *page_name, end: return ret; } + +static +int read_proc_meminfo_field(const char *field, size_t *value) +{ + int ret; + FILE *proc_meminfo; + char name[PROC_MEMINFO_FIELD_MAX_NAME_LEN] = {}; + + proc_meminfo = fopen(PROC_MEMINFO_PATH, "r"); + if (!proc_meminfo) { + PERROR("Failed to fopen() " PROC_MEMINFO_PATH); + ret = -1; + goto fopen_error; + } + + /* + * Read the contents of /proc/meminfo line by line to find the right + * field. + */ + while (!feof(proc_meminfo)) { + unsigned long value_kb; + + ret = fscanf(proc_meminfo, + "%" MAX_NAME_LEN_SCANF_IS_A_BROKEN_API "s %lu kB\n", + name, &value_kb); + if (ret == EOF) { + /* + * fscanf() returning EOF can indicate EOF or an error. + */ + if (ferror(proc_meminfo)) { + PERROR("Failed to parse " PROC_MEMINFO_PATH); + } + break; + } + + if (ret == 2 && strcmp(name, field) == 0) { + /* + * This number is displayed in kilo-bytes. Return the + * number of bytes. + */ + *value = ((size_t) value_kb) * 1024; + ret = 0; + goto found; + } + } + /* Reached the end of the file without finding the right field. */ + ret = -1; + +found: + fclose(proc_meminfo); +fopen_error: + return ret; +} + +/* + * Returns an estimate of the number of bytes of memory available based on the + * the information in `/proc/meminfo`. The number returned by this function is + * a best guess. + */ +LTTNG_HIDDEN +int utils_get_memory_available(size_t *value) +{ + return read_proc_meminfo_field(PROC_MEMINFO_MEMAVAILABLE_LINE, value); +} + +/* + * Returns the total size of the memory on the system in bytes based on the + * the information in `/proc/meminfo`. + */ +LTTNG_HIDDEN +int utils_get_memory_total(size_t *value) +{ + return read_proc_meminfo_field(PROC_MEMINFO_MEMTOTAL_LINE, value); +} diff --git a/src/common/utils.h b/src/common/utils.h index 1221f0eec..9e3fa60cd 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -61,5 +61,7 @@ int utils_create_lock_file(const char *filepath); int utils_recursive_rmdir(const char *path); int utils_truncate_stream_file(int fd, off_t length); int utils_show_help(int section, const char *page_name, const char *help_msg); +int utils_get_memory_available(size_t *value); +int utils_get_memory_total(size_t *value); #endif /* _COMMON_UTILS_H */ diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index b57be223a..165fef4dc 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -287,6 +287,50 @@ end: return ret; } +static int check_enough_available_memory(size_t num_bytes_requested_per_cpu) +{ + int ret; + long num_cpu; + size_t best_mem_info; + size_t num_bytes_requested_total; + + /* + * Get the number of CPU currently online to compute the amount of + * memory needed to create a buffer for every CPU. + */ + num_cpu = sysconf(_SC_NPROCESSORS_ONLN); + if (num_cpu == -1) { + goto error; + } + + num_bytes_requested_total = num_bytes_requested_per_cpu * num_cpu; + + /* + * Try to get the `MemAvail` field of `/proc/meminfo`. This is the most + * reliable estimate we can get but it is only exposed by the kernel + * since 3.14. (See Linux kernel commit: + * 34e431b0ae398fc54ea69ff85ec700722c9da773) + */ + ret = utils_get_memory_available(&best_mem_info); + if (ret >= 0) { + goto success; + } + + /* + * As a backup plan, use `MemTotal` field of `/proc/meminfo`. This + * is a sanity check for obvious user error. + */ + ret = utils_get_memory_total(&best_mem_info); + if (ret >= 0) { + goto success; + } + +error: + return -1; +success: + return best_mem_info >= num_bytes_requested_total; +} + /* * Try connect to session daemon with sock_path. * @@ -1475,6 +1519,7 @@ int lttng_enable_channel(struct lttng_handle *handle, struct lttng_channel *in_chan) { struct lttcomm_session_msg lsm; + size_t total_buffer_size_needed_per_cpu = 0; /* NULL arguments are forbidden. No default values. */ if (handle == NULL || in_chan == NULL) { @@ -1510,6 +1555,16 @@ int lttng_enable_channel(struct lttng_handle *handle, memcpy(&lsm.u.channel.extended, extended, sizeof(*extended)); } + /* + * Verify that the amount of memory required to create the requested + * buffer is available on the system at the moment. + */ + total_buffer_size_needed_per_cpu = lsm.u.channel.chan.attr.num_subbuf * + lsm.u.channel.chan.attr.subbuf_size; + if (!check_enough_available_memory(total_buffer_size_needed_per_cpu)) { + return -LTTNG_ERR_NOMEM; + } + lsm.cmd_type = LTTNG_ENABLE_CHANNEL; lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain); diff --git a/tests/regression/kernel/test_channel b/tests/regression/kernel/test_channel new file mode 100755 index 000000000..cb0fa5183 --- /dev/null +++ b/tests/regression/kernel/test_channel @@ -0,0 +1,74 @@ +#!/bin/bash +# +# Copyright (C) - 2018 Francis Deslauriers +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License, version 2 only, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +TEST_DESC="Kernel tracer - Channel configuration" + +CURDIR=$(dirname $0)/ +TESTDIR=$CURDIR/../.. +NUM_TESTS=8 + +source $TESTDIR/utils/utils.sh + +function test_channel_buffer() +{ + TRACE_PATH=$(mktemp -d) + SESSION_NAME="test_session_name" + CHANNEL_NAME="test_channel_name" + create_lttng_session_ok "$SESSION_NAME" "$TRACE_PATH" + + # Try to create a tiny buffer. + lttng_enable_kernel_channel_ok "$SESSION_NAME" "$CHANNEL_NAME" --subbuf-size=4k --num-subbuf=1 + + destroy_lttng_session_ok "$SESSION_NAME" + + rm -rf "$TRACE_PATH" +} + +function test_channel_buffer_too_large() +{ + TRACE_PATH=$(mktemp -d) + SESSION_NAME="test_session_name" + CHANNEL_NAME="test_channel_name" + create_lttng_session_ok "$SESSION_NAME" "$TRACE_PATH" + + # Try to create a buffer larger than memory. This testcase will need to + # be adjusted if someone has a computer with 1024*1000 GB of ram. + lttng_enable_kernel_channel_fail "$SESSION_NAME" "$CHANNEL_NAME" --subbuf-size=1000G --num-subbuf=1024 + + destroy_lttng_session_ok "$SESSION_NAME" + + rm -rf "$TRACE_PATH" +} + +plan_tests $NUM_TESTS +print_test_banner "$TEST_DESC" + +if [ "$(id -u)" == "0" ]; then + isroot=1 +else + isroot=0 +fi + +skip $isroot "Root access is needed. Skipping all tests." $NUM_TESTS || +{ + start_lttng_sessiond + + test_channel_buffer + test_channel_buffer_too_large + + stop_lttng_sessiond +} diff --git a/tests/root_regression b/tests/root_regression index 1fc5bbfe8..f1f661997 100644 --- a/tests/root_regression +++ b/tests/root_regression @@ -1,4 +1,5 @@ regression/kernel/test_all_events +regression/kernel/test_channel regression/kernel/test_event_basic regression/kernel/test_syscall regression/kernel/test_clock_override -- 2.34.1