From 13dd7782d52be2dac47be5116c00d3a2ba0ad626 Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Thu, 27 Jan 2022 14:22:22 -0500 Subject: [PATCH] Fix: conversion from KB to bytes overflow on arm32 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== On enable channel the memory available check fails on arm32 when available memory, in bytes, is larger than 2^32. Cause ===== `read_proc_meminfo_field` converts the read value (in KB) to bytes and stores it into a size_t variable. On the system running the reproducer the value of the `value_kb` variable is 4839692, yielding an overflow when multiplied with 1024 since `size_t` is 32 bit long. `size_t` can be larger in certain situation (i.e LARGEFILE) but this is irrelevant to the problem at hand. Solution ======== Convert all the checks to use uint64_t. Known drawbacks ========= None. References ========== The multiplication overflow check scheme is borrowed from `src/common/time.c` Signed-off-by: Jonathan Rajotte Signed-off-by: Jérémie Galarneau Change-Id: I067da25659ab4115e5494e48aab45a1c35f56652 --- src/common/utils.cpp | 18 ++++++++---- src/common/utils.h | 4 +-- src/lib/lttng-ctl/lttng-ctl.cpp | 52 ++++++++++++++++++++++++++------- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/common/utils.cpp b/src/common/utils.cpp index 410f8ec87..5e116a9ab 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -7,6 +7,7 @@ */ #include "common/macros.h" +#include #define _LGPL_SOURCE #include #include @@ -1041,7 +1042,7 @@ end: } static -int read_proc_meminfo_field(const char *field, size_t *value) +int read_proc_meminfo_field(const char *field, uint64_t *value) { int ret; FILE *proc_meminfo; @@ -1059,10 +1060,10 @@ int read_proc_meminfo_field(const char *field, size_t *value) * field. */ while (!feof(proc_meminfo)) { - unsigned long value_kb; + uint64_t value_kb; ret = fscanf(proc_meminfo, - "%" MAX_NAME_LEN_SCANF_IS_A_BROKEN_API "s %lu kB\n", + "%" MAX_NAME_LEN_SCANF_IS_A_BROKEN_API "s %" SCNu64 " kB\n", name, &value_kb); if (ret == EOF) { /* @@ -1079,7 +1080,12 @@ int read_proc_meminfo_field(const char *field, size_t *value) * This number is displayed in kilo-bytes. Return the * number of bytes. */ - *value = ((size_t) value_kb) * 1024; + if (value_kb > UINT64_MAX / 1024) { + ERR("Overflow on kb to bytes conversion"); + break; + } + + *value = value_kb * 1024; ret = 0; goto found; } @@ -1098,7 +1104,7 @@ fopen_error: * the information in `/proc/meminfo`. The number returned by this function is * a best guess. */ -int utils_get_memory_available(size_t *value) +int utils_get_memory_available(uint64_t *value) { return read_proc_meminfo_field(PROC_MEMINFO_MEMAVAILABLE_LINE, value); } @@ -1107,7 +1113,7 @@ int utils_get_memory_available(size_t *value) * Returns the total size of the memory on the system in bytes based on the * the information in `/proc/meminfo`. */ -int utils_get_memory_total(size_t *value) +int utils_get_memory_total(uint64_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 791b1098e..8b6003b92 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -50,8 +50,8 @@ 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); +int utils_get_memory_available(uint64_t *value); +int utils_get_memory_total(uint64_t *value); int utils_change_working_directory(const char *path); enum lttng_error_code utils_user_id_from_name( const char *user_name, uid_t *user_id); diff --git a/src/lib/lttng-ctl/lttng-ctl.cpp b/src/lib/lttng-ctl/lttng-ctl.cpp index 684cf4669..1743e7546 100644 --- a/src/lib/lttng-ctl/lttng-ctl.cpp +++ b/src/lib/lttng-ctl/lttng-ctl.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -290,12 +291,14 @@ end: return ret; } -static int check_enough_available_memory(size_t num_bytes_requested_per_cpu) +static enum lttng_error_code check_enough_available_memory( + uint64_t num_bytes_requested_per_cpu) { int ret; + enum lttng_error_code ret_code; long num_cpu; - size_t best_mem_info; - size_t num_bytes_requested_total; + uint64_t best_mem_info; + uint64_t num_bytes_requested_total; /* * Get the number of CPU currently online to compute the amount of @@ -303,10 +306,18 @@ static int check_enough_available_memory(size_t num_bytes_requested_per_cpu) */ num_cpu = sysconf(_SC_NPROCESSORS_ONLN); if (num_cpu == -1) { - goto error; + ret_code = LTTNG_ERR_FATAL; + goto end; + } + + if (num_bytes_requested_per_cpu > UINT64_MAX / (uint64_t) num_cpu) { + /* Overflow */ + ret_code = LTTNG_ERR_OVERFLOW; + goto end; } - num_bytes_requested_total = num_bytes_requested_per_cpu * num_cpu; + num_bytes_requested_total = + num_bytes_requested_per_cpu * (uint64_t) num_cpu; /* * Try to get the `MemAvail` field of `/proc/meminfo`. This is the most @@ -328,10 +339,18 @@ static int check_enough_available_memory(size_t num_bytes_requested_per_cpu) goto success; } -error: - return -1; + /* No valid source of information. */ + ret_code = LTTNG_ERR_NOMEM; + goto end; + success: - return best_mem_info >= num_bytes_requested_total; + if (best_mem_info >= num_bytes_requested_total) { + ret_code = LTTNG_OK; + } else { + ret_code = LTTNG_ERR_NOMEM; + } +end: + return ret_code; } /* @@ -1580,9 +1599,10 @@ void lttng_channel_destroy(struct lttng_channel *channel) int lttng_enable_channel(struct lttng_handle *handle, struct lttng_channel *in_chan) { + enum lttng_error_code ret_code; int ret; struct lttcomm_session_msg lsm; - size_t total_buffer_size_needed_per_cpu = 0; + uint64_t total_buffer_size_needed_per_cpu = 0; /* NULL arguments are forbidden. No default values. */ if (handle == NULL || in_chan == NULL) { @@ -1622,10 +1642,20 @@ int lttng_enable_channel(struct lttng_handle *handle, * Verify that the amount of memory required to create the requested * buffer is available on the system at the moment. */ + if (lsm.u.channel.chan.attr.num_subbuf > + UINT64_MAX / lsm.u.channel.chan.attr.subbuf_size) { + /* Overflow */ + ret = -LTTNG_ERR_OVERFLOW; + goto end; + } + 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; + ret_code = check_enough_available_memory( + total_buffer_size_needed_per_cpu); + if (ret_code != LTTNG_OK) { + ret = -ret_code; + goto end; } lsm.cmd_type = LTTNG_ENABLE_CHANNEL; -- 2.34.1