Fix: conversion from KB to bytes overflow on arm32

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 <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I067da25659ab4115e5494e48aab45a1c35f56652

diff --git a/src/common/utils.cpp b/src/common/utils.cpp
index 410f8ec872e92de06da8171a10df72257372be9b..5e116a9ab09513baa6809ed06671954c37156372 100644 (file)
--- a/src/common/utils.cpp
+++ b/src/common/utils.cpp
@@ -7,6 +7,7 @@
  */
 
 #include "common/macros.h"
+#include <stdint.h>
 #define _LGPL_SOURCE
 #include <ctype.h>
 #include <fcntl.h>
@@ -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 791b1098e79fefce2b72229986d564f76c652ad0..8b6003b926295696b5cba6e0848f1b14829dd92b 100644 (file)
--- 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 684cf466984eabc6fdd973a24a581f7cabaf28e9..1743e7546c1071713aff10d9f10c73ad30ee955a 100644 (file)
--- a/src/lib/lttng-ctl/lttng-ctl.cpp
+++ b/src/lib/lttng-ctl/lttng-ctl.cpp
@@ -14,6 +14,7 @@
 #include <grp.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdint.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -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;