From 4aee28c67b48f53b521e5512fc648f4ba2afabfb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 26 Jan 2019 15:42:35 +0100 Subject: [PATCH 1/2] alloc-util: whenever any of our alloca() wrappers is used to allocate overly large memory blocks, hit an assert() Of course, this should never happen, but let's better be safe than sorry, and abort rather than continue when a too large memory block is allocated, simply asa safety precaution. An early abort is better than continuing with a likely memory corruption later. --- src/basic/alloc-util.h | 49 +++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index ff7a46793a..893a1238ff 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -10,20 +10,28 @@ typedef void (*free_func_t)(void *p); +/* If for some reason more than 4M are allocated on the stack, let's abort immediately. It's better than + * proceeding and smashing the stack limits. Note that by default RLIMIT_STACK is 8M on Linux. */ +#define ALLOCA_MAX (4U*1024U*1024U) + #define new(t, n) ((t*) malloc_multiply(sizeof(t), (n))) #define new0(t, n) ((t*) calloc((n) ?: 1, sizeof(t))) -#define newa(t, n) \ - ({ \ - assert(!size_multiply_overflow(sizeof(t), n)); \ - (t*) alloca(sizeof(t)*(n)); \ +#define newa(t, n) \ + ({ \ + size_t _n_ = n; \ + assert(!size_multiply_overflow(sizeof(t), _n_)); \ + assert(sizeof(t)*_n_ <= ALLOCA_MAX); \ + (t*) alloca(sizeof(t)*_n_); \ }) -#define newa0(t, n) \ - ({ \ - assert(!size_multiply_overflow(sizeof(t), n)); \ - (t*) alloca0(sizeof(t)*(n)); \ +#define newa0(t, n) \ + ({ \ + size_t _n_ = n; \ + assert(!size_multiply_overflow(sizeof(t), _n_)); \ + assert(sizeof(t)*_n_ <= ALLOCA_MAX); \ + (t*) alloca0(sizeof(t)*_n_); \ }) #define newdup(t, p, n) ((t*) memdup_multiply(p, sizeof(t), (n))) @@ -51,16 +59,20 @@ void* memdup_suffix0(const void *p, size_t l) _alloc_(2); #define memdupa(p, l) \ ({ \ void *_q_; \ - _q_ = alloca(l); \ - memcpy(_q_, p, l); \ + size_t _l_ = l; \ + assert(_l_ <= ALLOCA_MAX); \ + _q_ = alloca(_l_); \ + memcpy(_q_, p, _l_); \ }) #define memdupa_suffix0(p, l) \ ({ \ void *_q_; \ - _q_ = alloca(l + 1); \ - ((uint8_t*) _q_)[l] = 0; \ - memcpy(_q_, p, l); \ + size_t _l_ = l; \ + assert(_l_ <= ALLOCA_MAX); \ + _q_ = alloca(_l_ + 1); \ + ((uint8_t*) _q_)[_l_] = 0; \ + memcpy(_q_, p, _l_); \ }) static inline void freep(void *p) { @@ -116,6 +128,7 @@ void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size); ({ \ char *_new_; \ size_t _len_ = n; \ + assert(_len_ <= ALLOCA_MAX); \ _new_ = alloca(_len_); \ (void *) memset(_new_, 0, _len_); \ }) @@ -125,16 +138,18 @@ void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size); ({ \ void *_ptr_; \ size_t _mask_ = (align) - 1; \ - _ptr_ = alloca((size) + _mask_); \ + size_t _size_ = size; \ + assert(_size_ <= ALLOCA_MAX); \ + _ptr_ = alloca(_size_ + _mask_); \ (void*)(((uintptr_t)_ptr_ + _mask_) & ~_mask_); \ }) #define alloca0_align(size, align) \ ({ \ void *_new_; \ - size_t _size_ = (size); \ - _new_ = alloca_align(_size_, (align)); \ - (void*)memset(_new_, 0, _size_); \ + size_t _xsize_ = (size); \ + _new_ = alloca_align(_xsize_, (align)); \ + (void*)memset(_new_, 0, _xsize_); \ }) /* Takes inspiration from Rusts's Option::take() method: reads and returns a pointer, but at the same time resets it to From 6e9417f5b4f29938fab1eee2b5edf596cc580452 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 26 Jan 2019 15:52:18 +0100 Subject: [PATCH 2/2] tree-wide: use newa() instead of alloca() wherever we can Typesafety is nice. And this way we can take benefit of the new size assert() the previous commit added. --- src/basic/path-util.h | 2 +- src/basic/process-util.h | 4 ++-- src/basic/string-util.h | 3 ++- src/journal/audit-type.h | 3 ++- src/journal/journal-send.c | 4 ++-- src/journal/journald-audit.c | 2 +- src/libsystemd-network/network-internal.c | 2 +- src/libsystemd/sd-bus/bus-objects.c | 12 ++++++------ src/libsystemd/sd-bus/bus-socket.c | 2 +- src/network/netdev/netdev.c | 2 +- src/nspawn/nspawn-network.c | 2 +- src/resolve/resolved-dns-dnssec.c | 2 +- src/tmpfiles/tmpfiles.c | 2 +- 13 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 094aa47c01..86c5a577cb 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -131,7 +131,7 @@ char *prefix_root(const char *root, const char *path); _ret = _path; \ else { \ _l = strlen(_root) + 1 + strlen(_path) + 1; \ - _n = alloca(_l); \ + _n = newa(char, _l); \ _p = stpcpy(_n, _root); \ while (_p > _n && _p[-1] == '/') \ _p--; \ diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 496e14d3de..c85ea30ecc 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -24,8 +24,8 @@ if (_pid_ == 0) { \ _r_ = ("/proc/self/" field); \ } else { \ - _r_ = alloca(STRLEN("/proc/") + DECIMAL_STR_MAX(pid_t) + 1 + sizeof(field)); \ - sprintf((char*) _r_, "/proc/"PID_FMT"/" field, _pid_); \ + _r_ = newa(char, STRLEN("/proc/") + DECIMAL_STR_MAX(pid_t) + 1 + sizeof(field)); \ + sprintf((char*) _r_, "/proc/"PID_FMT"/" field, _pid_); \ } \ _r_; \ }) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index a5b5a16a5d..38070abb22 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -6,6 +6,7 @@ #include #include +#include "alloc-util.h" #include "macro.h" /* What is interpreted as whitespace? */ @@ -111,7 +112,7 @@ char *strjoin_real(const char *x, ...) _sentinel_; size_t _i_; \ for (_i_ = 0; _i_ < ELEMENTSOF(_appendees_) && _appendees_[_i_]; _i_++) \ _len_ += strlen(_appendees_[_i_]); \ - _p_ = _d_ = alloca(_len_ + 1); \ + _p_ = _d_ = newa(char, _len_ + 1); \ for (_i_ = 0; _i_ < ELEMENTSOF(_appendees_) && _appendees_[_i_]; _i_++) \ _p_ = stpcpy(_p_, _appendees_[_i_]); \ *_p_ = 0; \ diff --git a/src/journal/audit-type.h b/src/journal/audit-type.h index 069a883490..98e5c39420 100644 --- a/src/journal/audit-type.h +++ b/src/journal/audit-type.h @@ -4,6 +4,7 @@ #include #include +#include "alloc-util.h" #include "macro.h" const char *audit_type_to_string(int type); @@ -15,7 +16,7 @@ int audit_type_from_string(const char *s); const char *_s_; \ _s_ = audit_type_to_string(type); \ if (!_s_) { \ - _s_ = alloca(STRLEN("AUDIT") + DECIMAL_STR_MAX(int)); \ + _s_ = newa(char, STRLEN("AUDIT") + DECIMAL_STR_MAX(int)); \ sprintf((char*) _s_, "AUDIT%04i", type); \ } \ _s_; \ diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c index 87056435fc..8618454131 100644 --- a/src/journal/journal-send.c +++ b/src/journal/journal-send.c @@ -30,7 +30,7 @@ const char *_func = (func); \ char **_f = &(f); \ _fl = strlen(_func) + 1; \ - *_f = alloca(_fl + 10); \ + *_f = newa(char, _fl + 10); \ memcpy(*_f, "CODE_FUNC=", 10); \ memcpy(*_f + 10, _func, _fl); \ } while (false) @@ -403,7 +403,7 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve identifier = strempty(identifier); l = strlen(identifier); - header = alloca(l + 1 + 1 + 2 + 2 + 2 + 2 + 2); + header = newa(char, l + 1 + 1 + 2 + 2 + 2 + 2 + 2); memcpy(header, identifier, l); header[l++] = '\n'; diff --git a/src/journal/journald-audit.c b/src/journal/journald-audit.c index 345e43ef44..accbad4180 100644 --- a/src/journal/journald-audit.c +++ b/src/journal/journald-audit.c @@ -162,7 +162,7 @@ static int map_generic_field(const char *prefix, const char **p, struct iovec ** if (e <= *p || e >= *p + 16) return 0; - c = alloca(strlen(prefix) + (e - *p) + 2); + c = newa(char, strlen(prefix) + (e - *p) + 2); t = stpcpy(c, prefix); for (f = *p; f < e; f++) { diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c index b3b134d650..0348e7fa9d 100644 --- a/src/libsystemd-network/network-internal.c +++ b/src/libsystemd-network/network-internal.c @@ -52,7 +52,7 @@ int net_get_unique_predictable_data(sd_device *device, uint64_t *result) { l = strlen(name); sz = sizeof(sd_id128_t) + l; - v = alloca(sz); + v = newa(uint8_t, sz); /* fetch some persistent data unique to this machine */ r = sd_id128_get_machine((sd_id128_t*) v); diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index d0538104ae..58329f3fe7 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -1149,7 +1149,7 @@ static int object_manager_serialize_path_and_fallbacks( return 0; /* Second, add fallback vtables registered for any of the prefixes */ - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { r = object_manager_serialize_path(bus, reply, prefix, path, true, error); if (r < 0) @@ -1500,7 +1500,7 @@ static int bus_find_parent_object_manager(sd_bus *bus, struct node **out, const if (!n) { char *prefix; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { n = hashmap_get(bus->nodes, prefix); if (n) @@ -2114,7 +2114,7 @@ _public_ int sd_bus_emit_properties_changed_strv( if (bus->nodes_modified) continue; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { r = emit_properties_changed_on_interface(bus, prefix, path, interface, true, &found_interface, names); if (r != 0) @@ -2291,7 +2291,7 @@ static int object_added_append_all(sd_bus *bus, sd_bus_message *m, const char *p if (bus->nodes_modified) return 0; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { r = object_added_append_all_prefix(bus, m, s, prefix, path, true); if (r < 0) @@ -2462,7 +2462,7 @@ static int object_removed_append_all(sd_bus *bus, sd_bus_message *m, const char if (bus->nodes_modified) return 0; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { r = object_removed_append_all_prefix(bus, m, s, prefix, path, true); if (r < 0) @@ -2626,7 +2626,7 @@ static int interfaces_added_append_one( if (bus->nodes_modified) return 0; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); OBJECT_PATH_FOREACH_PREFIX(prefix, path) { r = interfaces_added_append_one_prefix(bus, m, prefix, path, interface, true); if (r != 0) diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index ed185131b8..30d6455b6f 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -981,7 +981,7 @@ int bus_socket_write_message(sd_bus *bus, sd_bus_message *m, size_t *idx) { return r; n = m->n_iovec * sizeof(struct iovec); - iov = alloca(n); + iov = newa(struct iovec, n); memcpy_safe(iov, m->iovec, n); j = 0; diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 65959f4a03..0263917468 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -481,7 +481,7 @@ int netdev_get_mac(const char *ifname, struct ether_addr **ret) { l = strlen(ifname); sz = sizeof(sd_id128_t) + l; - v = alloca(sz); + v = newa(uint8_t, sz); /* fetch some persistent data unique to the machine */ r = sd_id128_get_machine((sd_id128_t*) v); diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 9d0f8a9956..c028b5755a 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -67,7 +67,7 @@ static int generate_mac( if (idx > 0) sz += sizeof(idx); - v = alloca(sz); + v = newa(uint8_t, sz); /* fetch some persistent data unique to the host */ r = sd_id128_get_machine((sd_id128_t*) v); diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 335fd47780..14acc4e77d 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -402,7 +402,7 @@ static int dnssec_ecdsa_verify( if (rrsig->rrsig.signature_size != key_size * 2) return -EINVAL; - q = alloca(key_size*2 + 1); + q = newa(uint8_t, key_size*2 + 1); q[0] = 0x04; /* Prepend 0x04 to indicate an uncompressed key */ memcpy(q+1, dnskey->dnskey.key, key_size*2); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 03a9e1d1f0..b66765b407 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -3130,7 +3130,7 @@ static int link_parent(ItemArray *a) { return 0; path = a->items[0].path; - prefix = alloca(strlen(path) + 1); + prefix = newa(char, strlen(path) + 1); PATH_FOREACH_PREFIX(prefix, path) { ItemArray *j;