diff --git a/0150-filter_qualify-free-allocated-data-on-the-error-path.patch b/0150-filter_qualify-free-allocated-data-on-the-error-path.patch new file mode 100644 index 0000000..7043b2f --- /dev/null +++ b/0150-filter_qualify-free-allocated-data-on-the-error-path.patch @@ -0,0 +1,77 @@ +From a034f8a50cbe15d250457ed2eefbf9db059f724f Mon Sep 17 00:00:00 2001 +From: Eugene Syromyatnikov +Date: Wed, 18 Aug 2021 21:48:38 +0200 +Subject: [PATCH 147/150] filter_qualify: free allocated data on the error path + exit of parse_poke_token + +While not terribly required due to the fact that issues with option +parsing lead to program termination, these changes avoid leaking data +allocated in the function's scope and not stored elsewhere, which might +come handy if it ever be used dynamically during the runtime. + +This also has been reported as resource leaks by covscan, and these +changes should calm it. + +* src/filter_qualify.c (parse_poke_token): Go to err label instead of +returning right away; free poke->data, poke, and str_tokenized before +returning false. + +References: https://bugzilla.redhat.com/show_bug.cgi?id=1995509 +--- + src/filter_qualify.c | 18 ++++++++++++------ + 1 file changed, 12 insertions(+), 6 deletions(-) + +diff --git a/src/filter_qualify.c b/src/filter_qualify.c +index df05496..a1a6471 100644 +--- a/src/filter_qualify.c ++++ b/src/filter_qualify.c +@@ -169,34 +169,40 @@ parse_poke_token(const char *input, struct inject_opts *fopts, bool isenter) + poke->is_enter = isenter; + + if ((val = STR_STRIP_PREFIX(token, "@arg")) == token) +- return false; ++ goto err; + if ((val[0] >= '1') && (val[0] <= '7')) { + poke->arg_no = val[0] - '0'; + } else { +- return false; ++ goto err; + } + if (val[1] != '=') +- return false; ++ goto err; + val += 2; + + data_len = strlen(val); + if ((data_len == 0) || (data_len % 2) || (data_len > 2048)) +- return false; ++ goto err; + data_len /= 2; + poke->data_len = data_len; + poke->data = xmalloc(data_len); + + for (size_t i = 0; i < data_len; i++) + if (sscanf(&val[2 * i], "%2hhx", &poke->data[i]) != 1) +- return false; ++ goto err; + + if (poke_add(fopts->data.poke_idx, poke)) +- return false; ++ goto err; + } + free(str_tokenized); + + fopts->data.flags |= flag; + return true; ++ ++err: ++ free(poke->data); ++ free(poke); ++ free(str_tokenized); ++ return false; + } + + static bool +-- +2.1.4 + diff --git a/0151-macros-expand-BIT-macros-add-MASK-macros-add-_SAFE-m.patch b/0151-macros-expand-BIT-macros-add-MASK-macros-add-_SAFE-m.patch new file mode 100644 index 0000000..6ee45fb --- /dev/null +++ b/0151-macros-expand-BIT-macros-add-MASK-macros-add-_SAFE-m.patch @@ -0,0 +1,70 @@ +From 3f3dd44f1964c54b55e8c84343579bd7c1924df5 Mon Sep 17 00:00:00 2001 +From: Eugene Syromyatnikov +Date: Wed, 18 Aug 2021 21:49:12 +0200 +Subject: [PATCH 148/150] macros: expand BIT macros, add MASK macros; add + *_SAFE macros + +These macros might make reading a code that often converts between powers +of 2 and values/masks a bit easier; moreover, the *_SAFE versions should +help in cases where the shift values are expected to be equal to the type +bit width (which lead to UB otherwise). + +Switching from BIT to BIT32 should also clarify bitness, which may be somewhat +murky at times (cf. printxval, printflags, and printxvals). + +* src/macros.h [!BIT] (BIT): Rename to... +[!BIT32] (BIT32): ...this. +[!BIT64] (BIT64): New macro. +[!MASK32] (MASK32): Likewise. +[!MASK64] (MASK64): Likewise. +(BIT32_SAFE, BIT64_SAFE, MASK32_SAFE, MASK64_SAFE): New macros. +(FLAG): Use BIT32. +--- + src/macros.h | 30 +++++++++++++++++++++++++++--- + 1 file changed, 27 insertions(+), 3 deletions(-) + +diff --git a/src/macros.h b/src/macros.h +index 467f5d0..2d7a83d 100644 +--- a/src/macros.h ++++ b/src/macros.h +@@ -78,10 +78,34 @@ is_filled(const char *ptr, char fill, size_t size) + # define IS_ARRAY_ZERO(arr_) \ + is_filled((const char *) (arr_), 0, sizeof(arr_) + MUST_BE_ARRAY(arr_)) + +-# ifndef BIT +-# define BIT(x_) (1U << (x_)) ++# ifndef BIT32 ++# define BIT32(x_) (1U << (x_)) + # endif + +-# define FLAG(name_) name_ = BIT(name_##_BIT) ++# ifndef BIT64 ++# define BIT64(x_) (1ULL << (x_)) ++# endif ++ ++# ifndef MASK32 ++# define MASK32(x_) (BIT32(x_) - 1U) ++# endif ++ ++# ifndef MASK64 ++# define MASK64(x_) (BIT64(x_) - 1ULL) ++# endif ++ ++/* ++ * "Safe" versions that avoid UB for values that are >= type bit size ++ * (the usually expected behaviour of the bit shift in that case is zero, ++ * but at least powerpc is notorious for returning the input value when shift ++ * by 64 bits is performed). ++ */ ++ ++# define BIT32_SAFE(x_) ((x_) < 32 ? BIT32(x_) : 0) ++# define BIT64_SAFE(x_) ((x_) < 64 ? BIT64(x_) : 0) ++# define MASK32_SAFE(x_) (BIT32_SAFE(x_) - 1U) ++# define MASK64_SAFE(x_) (BIT64_SAFE(x_) - 1ULL) ++ ++# define FLAG(name_) name_ = BIT32(name_##_BIT) + + #endif /* !STRACE_MACROS_H */ +-- +2.1.4 + diff --git a/0152-trie-use-BIT-and-MASK-macros.patch b/0152-trie-use-BIT-and-MASK-macros.patch new file mode 100644 index 0000000..91c6fe3 --- /dev/null +++ b/0152-trie-use-BIT-and-MASK-macros.patch @@ -0,0 +1,151 @@ +From 8ef5456338a947944cc03b95c22c837af5884ddc Mon Sep 17 00:00:00 2001 +From: Eugene Syromyatnikov +Date: Wed, 18 Aug 2021 21:51:22 +0200 +Subject: [PATCH 149/150] trie: use BIT* and MASK* macros + +This makes reading the code a bit easier. It also solves some issues +where there is a hypothertical possibility of having bit shifts of size +64, by virtue of using the *_SAFE macros (that should silence some +reported "left shifting by more than 63 bits has undefined behavior" +covscan issues). + +* src/trie.c (trie_create): Use BIT32, MASK64. +(trie_create_data_block): Use BIT32, change iterator variable type +to size_t. +(trie_get_node): Use BIT64, MASK64. +(trie_data_block_calc_pos): Use BIT32, MASK64, MASK64_SAFE. +(trie_iterate_keys_node): Use BIT64, MASK64, MASK64_SAFE. +(trie_free_node): Use BIT64. +--- + src/trie.c | 34 +++++++++++++++++----------------- + 1 file changed, 17 insertions(+), 17 deletions(-) + +diff --git a/src/trie.c b/src/trie.c +index 586ff25..0a231e4 100644 +--- a/src/trie.c ++++ b/src/trie.c +@@ -15,6 +15,7 @@ + #include + + #include "trie.h" ++#include "macros.h" + #include "xmalloc.h" + + static const uint8_t ptr_sz_lg = (sizeof(void *) == 8 ? 6 : 5); +@@ -87,7 +88,7 @@ trie_create(uint8_t key_size, uint8_t item_size_lg, uint8_t node_key_bits, + / t->node_key_bits; + + if (item_size_lg != 6) +- t->empty_value &= (((uint64_t) 1 << (1 << t->item_size_lg)) - 1); ++ t->empty_value &= MASK64(BIT32(t->item_size_lg)); + + return t; + } +@@ -96,8 +97,8 @@ static void * + trie_create_data_block(struct trie *t) + { + uint64_t fill_value = t->empty_value; +- for (int i = 1; i < 1 << (6 - t->item_size_lg); i++) { +- fill_value <<= (1 << t->item_size_lg); ++ for (size_t i = 1; i < BIT32(6 - t->item_size_lg); i++) { ++ fill_value <<= BIT32(t->item_size_lg); + fill_value |= t->empty_value; + } + +@@ -105,7 +106,7 @@ trie_create_data_block(struct trie *t) + if (sz < 6) + sz = 6; + +- size_t count = 1 << (sz - 6); ++ size_t count = BIT32(sz - 6); + uint64_t *data_block = xcalloc(count, 8); + + for (size_t i = 0; i < count; i++) +@@ -119,7 +120,7 @@ trie_get_node(struct trie *t, uint64_t key, bool auto_create) + { + void **cur_node = &(t->data); + +- if (t->key_size < 64 && key > (uint64_t) 1 << t->key_size) ++ if (t->key_size < 64 && key > MASK64(t->key_size)) + return NULL; + + for (uint8_t cur_depth = 0; cur_depth <= t->max_depth; cur_depth++) { +@@ -133,13 +134,13 @@ trie_get_node(struct trie *t, uint64_t key, bool auto_create) + if (cur_depth == t->max_depth) + *cur_node = trie_create_data_block(t); + else +- *cur_node = xcalloc(1 << sz, 1); ++ *cur_node = xcalloc(BIT64(sz), 1); + } + + if (cur_depth == t->max_depth) + break; + +- size_t pos = (key >> offs) & ((1 << (sz - ptr_sz_lg)) - 1); ++ size_t pos = (key >> offs) & MASK64(sz - ptr_sz_lg); + cur_node = (((void **) (*cur_node)) + pos); + } + +@@ -152,7 +153,7 @@ trie_data_block_calc_pos(struct trie *t, uint64_t key, + { + uint64_t key_mask; + +- key_mask = (1 << t->data_block_key_bits) - 1; ++ key_mask = MASK64(t->data_block_key_bits); + *pos = (key & key_mask) >> (6 - t->item_size_lg); + + if (t->item_size_lg == 6) { +@@ -161,10 +162,10 @@ trie_data_block_calc_pos(struct trie *t, uint64_t key, + return; + } + +- key_mask = (1 << (6 - t->item_size_lg)) - 1; +- *offs = (key & key_mask) * (1 << t->item_size_lg); ++ key_mask = MASK64(6 - t->item_size_lg); ++ *offs = (key & key_mask) << t->item_size_lg; + +- *mask = (((uint64_t) 1 << (1 << t->item_size_lg)) - 1) << *offs; ++ *mask = MASK64_SAFE(BIT32(t->item_size_lg)) << *offs; + } + + bool +@@ -211,7 +212,7 @@ trie_iterate_keys_node(struct trie *t, + return 0; + + if (t->key_size < 64) { +- uint64_t key_max = ((uint64_t) 1 << t->key_size) - 1; ++ uint64_t key_max = MASK64(t->key_size); + if (end > key_max) + end = key_max; + } +@@ -228,15 +229,14 @@ trie_iterate_keys_node(struct trie *t, + t->key_size : + trie_get_node_bit_offs(t, depth - 1); + +- uint64_t first_key_in_node = start & +- (uint64_t) -1 << parent_node_bit_off; ++ uint64_t first_key_in_node = start & ~MASK64_SAFE(parent_node_bit_off); + + uint8_t node_bit_off = trie_get_node_bit_offs(t, depth); + uint8_t node_key_bits = parent_node_bit_off - node_bit_off; +- uint64_t mask = ((uint64_t) 1 << (node_key_bits)) - 1; ++ uint64_t mask = MASK64_SAFE(node_key_bits); + uint64_t start_index = (start >> node_bit_off) & mask; + uint64_t end_index = (end >> node_bit_off) & mask; +- uint64_t child_key_count = (uint64_t) 1 << node_bit_off; ++ uint64_t child_key_count = BIT64(node_bit_off); + + uint64_t count = 0; + +@@ -274,7 +274,7 @@ trie_free_node(struct trie *t, void *node, uint8_t depth) + if (depth >= t->max_depth) + goto free_node; + +- size_t sz = 1 << (trie_get_node_size(t, depth) - ptr_sz_lg); ++ size_t sz = BIT64(trie_get_node_size(t, depth) - ptr_sz_lg); + for (size_t i = 0; i < sz; i++) + trie_free_node(t, ((void **) node)[i], depth + 1); + +-- +2.1.4 + diff --git a/0153-tee-rewrite-num_params-access-in-tee_fetch_buf_data.patch b/0153-tee-rewrite-num_params-access-in-tee_fetch_buf_data.patch new file mode 100644 index 0000000..56aa8a1 --- /dev/null +++ b/0153-tee-rewrite-num_params-access-in-tee_fetch_buf_data.patch @@ -0,0 +1,52 @@ +From 3a68f90c2a5a208b475cc2014f85ae04541ec5b6 Mon Sep 17 00:00:00 2001 +From: Eugene Syromyatnikov +Date: Fri, 20 Aug 2021 21:31:01 +0200 +Subject: [PATCH 150/150] tee: rewrite num_params access in tee_fetch_buf_data + +Pointer to num_params field of the fetched structure is passed in a +separate function argument which provokes covscan complaints about +uninitialised accesses and also tingles my aliasing rules senses. +Rewrite to access it via the arg_struct argument which is fetched +earlier in the function flow. + +* src/tee.c (TEE_FETCH_BUF_DATA): Change &arg_.num_params +to offsetof(typeof(arg_), num_params). +(tee_fetch_buf_data): Accept offset of the num_params field instead +of pointer to it; reconstruct the num_params pointer using it. +--- + src/tee.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/src/tee.c b/src/tee.c +index f9eda52..d7e9b15 100644 +--- a/src/tee.c ++++ b/src/tee.c +@@ -33,7 +33,7 @@ struct tee_ioctl_shm_register_fd_data { + + #define TEE_FETCH_BUF_DATA(buf_, arg_, params_) \ + tee_fetch_buf_data(tcp, arg, &buf_, sizeof(arg_), \ +- &arg_, &arg_.num_params, \ ++ &arg_, offsetof(typeof(arg_), num_params), \ + params_) + + /* session id is printed as 0x%x in libteec */ +@@ -56,7 +56,7 @@ tee_fetch_buf_data(struct tcb *const tcp, + struct tee_ioctl_buf_data *buf, + size_t arg_size, + void *arg_struct, +- unsigned *num_params, ++ size_t num_params_offs, + uint64_t *params) + { + if (umove_or_printaddr(tcp, arg, buf)) +@@ -69,6 +69,7 @@ tee_fetch_buf_data(struct tcb *const tcp, + tee_print_buf(buf); + return RVAL_IOCTL_DECODED; + } ++ uint32_t *num_params = (uint32_t *) (arg_struct + num_params_offs); + if (entering(tcp) && + (arg_size + TEE_IOCTL_PARAM_SIZE(*num_params) != buf->buf_len)) { + /* +-- +2.1.4 + diff --git a/strace.spec b/strace.spec index 3268d3d..96961a7 100644 --- a/strace.spec +++ b/strace.spec @@ -1,7 +1,7 @@ Summary: Tracks and displays system calls associated with a running process Name: strace Version: 5.13 -Release: 2%{?dist} +Release: 3%{?dist} # The test suite is GPLv2+, all the rest is LGPLv2.1+. License: LGPL-2.1+ and GPL-2.0+ # Some distros require Group tag to be present, @@ -43,6 +43,14 @@ BuildRequires: pkgconfig(bluez) %define maybe_use_defattr %{?suse_version:%%defattr(-,root,root)} Patch141: 0141-tests-disable-sockopt-timestamp-on-new-glibc-with-__.patch +# v5.13-55-g6b2191f "filter_qualify: free allocated data on the error path exit of parse_poke_token" +Patch150: 0150-filter_qualify-free-allocated-data-on-the-error-path.patch +# v5.13-56-g80dc60c "macros: expand BIT macros, add MASK macros; add *_SAFE macros" +Patch151: 0151-macros-expand-BIT-macros-add-MASK-macros-add-_SAFE-m.patch +# v5.13-58-g94ae5c2 "trie: use BIT* and MASK* macros" +Patch152: 0152-trie-use-BIT-and-MASK-macros.patch +# v5.13-65-g41b753e "tee: rewrite num_params access in tee_fetch_buf_data" +Patch153: 0153-tee-rewrite-num_params-access-in-tee_fetch_buf_data.patch # Fallback definitions for make_build/make_install macros %{?!__make: %global __make %_bindir/make} @@ -64,6 +72,10 @@ received by a process. %setup -q %patch141 -p1 +%patch150 -p1 +%patch151 -p1 +%patch152 -p1 +%patch153 -p1 echo -n %version-%release > .tarball-version echo -n 2021 > .year @@ -111,6 +123,8 @@ echo 'END OF TEST SUITE INFORMATION' %{_mandir}/man1/* %changelog +* Mon Aug 23 2021 Eugene Syromiatnikov - 5.13-3 +- Address some issues reported by covscan (#1996691). * Tue Aug 10 2021 Mohan Boddu - 5.13-2 - Rebuilt for IMA sigs, glibc 2.34, aarch64 flags Related: rhbz#1991688