From 59a86cbc7c5d5640b16ca9d8a99be979b11a4c68 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 10:14:15 +0200 Subject: [PATCH 01/26] Makefile rule for static analyzer --- Makefile.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Makefile.in b/Makefile.in index e6dbd572..cf94e352 100644 --- a/Makefile.in +++ b/Makefile.in @@ -184,6 +184,14 @@ check: tests tests_run tests: $(tests_targets) tests_run: $(tests_targets_ok) +STATIC_CHECKERS := nullability.NullableDereferenced nullability.NullablePassedToNonnull nullability.NullableReturnedFromNonnull optin.portability.UnixAPI valist.CopyToSelf valist.Uninitialized valist.Unterminated +STATIC_SCAN_FLAGS := --force-analyze-debug-code -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS)) + +static-scan: + $(E)echo Running static code analysis + $(Q)$(MAKE) clean + $(Q)scan-build $(STATIC_SCAN_FLAGS) $(MAKE) -$(MAKEFLAGS) + tags: cd $(srcdir) ; etags -lc `find $(dirs) -name *.[chY]` From 124d860f648f4c1c080e77b5f070b97d094f5285 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 11:06:49 +0200 Subject: [PATCH 02/26] Filter: fixed omitted overflow check in EC constructor --- filter/f-inst.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/filter/f-inst.c b/filter/f-inst.c index 4b3c627b..3d185918 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -261,7 +261,7 @@ FID_MEMBER(enum ec_subtype, ecs, f1->ecs != f2->ecs, "ec subtype %s", ec_subtype_str(item->ecs)); - int check, ipv4_used; + int ipv4_used; u32 key, val; if (v1.type == T_INT) { @@ -279,21 +279,20 @@ val = v2.val.i; - if (ecs == EC_GENERIC) { - check = 0; RESULT(T_EC, ec, ec_generic(key, val)); - } - else if (ipv4_used) { - check = 1; RESULT(T_EC, ec, ec_ip4(ecs, key, val)); - } - else if (key < 0x10000) { - check = 0; RESULT(T_EC, ec, ec_as2(ecs, key, val)); - } - else { - check = 1; RESULT(T_EC, ec, ec_as4(ecs, key, val)); - } - - if (check && (val > 0xFFFF)) - runtime("Value %u > %u out of bounds in EC constructor", val, 0xFFFF); + if (ecs == EC_GENERIC) + RESULT(T_EC, ec, ec_generic(key, val)); + else if (ipv4_used) + if (val <= 0xFFFF) + RESULT(T_EC, ec, ec_ip4(ecs, key, val)); + else + runtime("4-byte value %u can't be used with IP-address key in extended community", val); + else if (key < 0x10000) + RESULT(T_EC, ec, ec_as2(ecs, key, val)); + else + if (val <= 0xFFFF) + RESULT(T_EC, ec, ec_as4(ecs, key, val)); + else + runtime("4-byte value %u can't be used with 4-byte ASN in extended community", val); } INST(FI_LC_CONSTRUCT, 3, 1) { From d607205486d7ea11f2cbf3dcc3d5e7e6b53f1d0f Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 10:28:23 +0200 Subject: [PATCH 03/26] Not calling memcpy with n=0. --- lib/string.h | 9 +++++++++ proto/bgp/bgp.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/string.h b/lib/string.h index d6ae5ef7..0f650178 100644 --- a/lib/string.h +++ b/lib/string.h @@ -72,6 +72,15 @@ bstrcmp(const char *s1, const char *s2) return !s2 - !s1; } +static inline void * +bmemcpy(void *dest, const void *src, size_t n) +{ + if (n) + return memcpy(dest, src, n); + else + return dest; +} + #define ROUTER_ID_64_LENGTH 23 #endif diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index dc63e13e..0529c45a 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -552,7 +552,7 @@ static inline void bgp_set_attr_data(ea_list **to, struct linpool *pool, uint code, uint flags, void *data, uint len) { struct adata *a = lp_alloc_adata(pool, len); - memcpy(a->data, data, len); + bmemcpy(a->data, data, len); bgp_set_attr(to, pool, code, flags, (uintptr_t) a); } From 8029ae527edde4d47a51b55efdbdea546296c5ef Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 1 Aug 2019 14:25:01 +0200 Subject: [PATCH 04/26] More assertion categories --- lib/birdlib.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/birdlib.h b/lib/birdlib.h index 5202b0c8..4adc42cd 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -162,10 +162,14 @@ void debug(const char *msg, ...); /* Printf to debug output */ #define DBG(x, y...) do { } while(0) #endif +#define ASSERT_DIE(x) do { if (!(x)) bug("Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) + #ifdef DEBUGGING -#define ASSERT(x) do { if (!(x)) bug("Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) +#define ASSERT(x) ASSERT_DIE(x) +#define ASSUME(x) ASSERT_DIE(x) #else #define ASSERT(x) do { if (!(x)) log(L_BUG "Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) +#define ASSUME(x) /* intentionally left blank */ #endif #ifdef DEBUGGING From bf9486bf20ee16af71e338ee690fc36805d98fe5 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Mon, 27 Apr 2020 22:33:10 +0200 Subject: [PATCH 05/26] Non-null function argument declaration --- lib/birdlib.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/birdlib.h b/lib/birdlib.h index 4adc42cd..caa26b94 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -72,6 +72,7 @@ static inline int u64_cmp(u64 i1, u64 i2) #define NORET __attribute__((noreturn)) #define UNUSED __attribute__((unused)) #define PACKED __attribute__((packed)) +#define NONNULL(...) __attribute__((nonnull((__VA_ARGS__)))) #ifndef HAVE_THREAD_LOCAL #define _Thread_local From 30ba7c1661a13d665ae0aaa4e40cb5ed24023450 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 11:31:03 +0200 Subject: [PATCH 06/26] Filter: Removed forgotten dead code --- filter/config.Y | 153 ------------------------------------------------ 1 file changed, 153 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 995f6cd4..49c59efc 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -185,159 +185,6 @@ f_generate_empty(struct f_dynamic_attr dyn) return f_new_inst(FI_EA_SET, f_new_inst(FI_CONSTANT, empty), dyn); } -#if 0 - -static inline struct f_inst * -f_generate_dpair(struct f_inst *t1, struct f_inst *t2) -{ - struct f_inst *rv; - - if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT)) { - if ((t1->val.type != T_INT) || (t2->val.type != T_INT)) - cf_error( "Can't operate with value of non-integer type in pair constructor"); - - check_u16(t1->a[1].i); - check_u16(t2->a[1].i); - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_PAIR, - .val.i = pair(t1->a[1].i, t2->a[1].i), - }; - } - else { - rv = f_new_inst(FI_PAIR_CONSTRUCT); - rv->a[0].p = t1; - rv->a[1].p = t2; - } - - return rv; -} - -static inline struct f_inst * -f_generate_ec(u16 kind, struct f_inst *tk, struct f_inst *tv) -{ - struct f_inst *rv; - int c1 = 0, c2 = 0, ipv4_used = 0; - u32 key = 0, val2 = 0; - - if (tk->fi_code == FI_CONSTANT) { - c1 = 1; - struct f_val *val = &(tk->val); - - if (val->type == T_INT) { - ipv4_used = 0; key = val->val.i; - } - else if (tk->val.type == T_QUAD) { - ipv4_used = 1; key = val->val.i; - } - else if ((val->type == T_IP) && ipa_is_ip4(val->val.ip)) { - ipv4_used = 1; key = ipa_to_u32(val->val.ip); - } - else - cf_error("Can't operate with key of non-integer/IPv4 type in EC constructor"); - } - - if (tv->fi_code == FI_CONSTANT) { - if (tv->val.type != T_INT) - cf_error("Can't operate with value of non-integer type in EC constructor"); - c2 = 1; - val2 = tv->val.val.i; - } - - if (c1 && c2) { - u64 ec; - - if (kind == EC_GENERIC) { - ec = ec_generic(key, val2); - } - else if (ipv4_used) { - check_u16(val2); - ec = ec_ip4(kind, key, val2); - } - else if (key < 0x10000) { - ec = ec_as2(kind, key, val2); - } - else { - check_u16(val2); - ec = ec_as4(kind, key, val2); - } - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_EC, - .val.ec = ec, - }; - } - else { - rv = f_new_inst(FI_EC_CONSTRUCT); - rv->aux = kind; - rv->a[0].p = tk; - rv->a[1].p = tv; - } - - return rv; -} - -static inline struct f_inst * -f_generate_lc(struct f_inst *t1, struct f_inst *t2, struct f_inst *t3) -{ - struct f_inst *rv; - - if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT) && (t3->fi_code == FI_CONSTANT)) { - if ((t1->val.type != T_INT) || (t2->val.type != T_INT) || (t3->val.type != T_INT)) - cf_error( "LC - Can't operate with value of non-integer type in tuple constructor"); - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_LC, - .val.lc = (lcomm) { t1->a[1].i, t2->a[1].i, t3->a[1].i }, - }; - } - else - { - rv = f_new_inst(FI_LC_CONSTRUCT); - rv->a[0].p = t1; - rv->a[1].p = t2; - rv->a[2].p = t3; - } - - return rv; -} - -static inline struct f_inst * -f_generate_path_mask(struct f_inst *t) -{ - uint len = 0; - uint dyn = 0; - for (const struct f_inst *tt = t; tt; tt = tt->next) { - if (tt->fi_code != FI_CONSTANT) - dyn++; - len++; - } - - if (dyn) { - struct f_inst *pmc = f_new_inst(FI_PATHMASK_CONSTRUCT); - pmc->a[0].p = t; - pmc->a[1].i = len; - return pmc; - } - - struct f_path_mask *pm = cfg_allocz(sizeof(struct f_path_mask) + len * sizeof(struct f_path_mask_item)); - - uint i = 0; - for (const struct f_inst *tt = t; tt; tt = tt->next) - pm->item[i++] = tt->val.val.pmi; - - pm->len = i; - struct f_inst *pmc = f_new_inst(FI_CONSTANT); - pmc->val = (struct f_val) { .type = T_PATH_MASK, .val.path_mask = pm, }; - - return pmc; -} - -#endif - /* * Remove all new lines and doubled whitespaces * and convert all tabulators to spaces From d65a926a67749f8e8ffb6df9b3e2e123669b0656 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 11:49:20 +0200 Subject: [PATCH 07/26] Filter: Don't alloc varargs array if its length would be zero --- filter/decl.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filter/decl.m4 b/filter/decl.m4 index efecb9a5..14b9329c 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -138,7 +138,7 @@ FID_IFCONST([[ } FID_IFCONST([[ const struct f_inst **items = NULL; - if (constargs) { + if (constargs && whati->varcount) { items = alloca(whati->varcount * sizeof(struct f_inst *)); const struct f_inst *child = fvar; for (uint i=0; child; i++) From a1b61a271af40a9d6ef0837424ab2c98d29f1575 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 12:29:04 +0200 Subject: [PATCH 08/26] IPv6 address parser: fail on incomplete addresses --- lib/ip.c | 5 +++++ lib/ip_test.c | 47 ++++++++++++++++++++++++++++++++++------------- test/birdtest.c | 5 ++++- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lib/ip.c b/lib/ip.c index 2d195160..fcc72caf 100644 --- a/lib/ip.c +++ b/lib/ip.c @@ -264,6 +264,9 @@ ip6_pton(const char *a, ip6_addr *o) int i, j, k, l, hfil; const char *start; + if (!a[0]) /* Empty string check */ + return 0; + if (a[0] == ':') /* Leading :: */ { if (a[1] != ':') @@ -333,6 +336,8 @@ ip6_pton(const char *a, ip6_addr *o) for (; i>=hfil; i--) words[i] = 0; } + else if (i != 8) /* Incomplete address */ + return 0; /* Convert the address to ip6_addr format */ for (i=0; i<4; i++) diff --git a/lib/ip_test.c b/lib/ip_test.c index fd70c957..36d10d68 100644 --- a/lib/ip_test.c +++ b/lib/ip_test.c @@ -13,25 +13,38 @@ #define IP4_MAX_LEN 16 static int -test_ipa_pton(void *out_, const void *in_, const void *expected_out_) +test_ip4_pton(void *out_, const void *in_, const void *expected_out_) +{ + ip_addr *out = out_; + const char *in = in_; + const ip_addr *expected_out = expected_out_; + ip4_addr ip4; + + if (expected_out) + { + bt_assert(ip4_pton(in, &ip4)); + *out = ipa_from_ip4(ip4); + return ipa_equal(*out, *expected_out); + } + else + return !ip4_pton(in, &ip4); + +} + +static int +test_ip6_pton(void *out_, const void *in_, const void *expected_out_) { ip_addr *out = out_; const char *in = in_; const ip_addr *expected_out = expected_out_; - if (ipa_is_ip4(*expected_out)) - { - ip4_addr ip4; - bt_assert(ip4_pton(in, &ip4)); - *out = ipa_from_ip4(ip4); - } - else + if (expected_out) { bt_assert(ip6_pton(in, out)); - /* ip_addr == ip6_addr */ + return ipa_equal(*out, *expected_out); } - - return ipa_equal(*out, *expected_out); + else + return !ip6_pton(in, out); } static int @@ -52,7 +65,7 @@ t_ip4_pton(void) }, }; - return bt_assert_batch(test_vectors, test_ipa_pton, bt_fmt_str, bt_fmt_ipa); + return bt_assert_batch(test_vectors, test_ip4_pton, bt_fmt_str, bt_fmt_ipa); } static int @@ -87,9 +100,17 @@ t_ip6_pton(void) .in = "2605:2700:0:3::4713:93e3", .out = & ipa_build6(0x26052700, 0x00000003, 0x00000000, 0x471393E3), }, + { + .in = "2605:2700:0:3:4713:93e3", + .out = NULL, + }, + { + .in = "2", + .out = NULL, + }, }; - return bt_assert_batch(test_vectors, test_ipa_pton, bt_fmt_str, bt_fmt_ipa); + return bt_assert_batch(test_vectors, test_ip6_pton, bt_fmt_str, bt_fmt_ipa); } static int diff --git a/test/birdtest.c b/test/birdtest.c index a092446a..641fd3c7 100644 --- a/test/birdtest.c +++ b/test/birdtest.c @@ -495,7 +495,10 @@ void bt_fmt_ipa(char *buf, size_t size, const void *data) { const ip_addr *ip = data; - bsnprintf(buf, size, "%I", *ip); + if (data) + bsnprintf(buf, size, "%I", *ip); + else + bsnprintf(buf, size, "(null)"); } int From a0d0a71a1828cce725c3132f8c243bf0c537786f Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 16:22:39 +0200 Subject: [PATCH 09/26] Expensive check declaration Intended to be run at every operation with complex data structures to check their consistency and validity. --- configure.ac | 13 +++++++++++++ lib/birdlib.h | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/configure.ac b/configure.ac index da8546a6..96f66644 100644 --- a/configure.ac +++ b/configure.ac @@ -24,6 +24,12 @@ AC_ARG_ENABLE([debug-generated], [enable_debug_generated=no] ) +AC_ARG_ENABLE([debug-expensive], + [AS_HELP_STRING([--enable-debug-expensive], [enable expensive consistency checks (implies --enable-debug) @<:@no@:>@])], + [], + [enable_debug_expensive=no] +) + AC_ARG_ENABLE([memcheck], [AS_HELP_STRING([--enable-memcheck], [check memory allocations when debugging @<:@yes@:>@])], [], @@ -72,6 +78,9 @@ AC_ARG_VAR([FLEX], [location of the Flex program]) AC_ARG_VAR([BISON], [location of the Bison program]) AC_ARG_VAR([M4], [location of the M4 program]) +if test "$enable_debug_expensive" = yes; then + enable_debug = yes +fi if test "$srcdir" = . ; then # Building in current directory => create obj directory holding all objects @@ -388,6 +397,10 @@ if test "$enable_debug" = yes ; then AC_CHECK_LIB([efence], [malloc]) fi fi + + if test "enable_debug_expensive" = yes ; then + AC_DEFINE([ENABLE_EXPENSIVE_CHECKS], [1], [Define to 1 if you want to run expensive consistency checks.]) + fi fi CLIENT=birdcl diff --git a/lib/birdlib.h b/lib/birdlib.h index caa26b94..23036c1b 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -165,14 +165,21 @@ void debug(const char *msg, ...); /* Printf to debug output */ #define ASSERT_DIE(x) do { if (!(x)) bug("Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) +#define EXPENSIVE_CHECK(x) /* intentionally left blank */ + #ifdef DEBUGGING #define ASSERT(x) ASSERT_DIE(x) #define ASSUME(x) ASSERT_DIE(x) +#ifdef ENABLE_EXPENSIVE_CHECKS +#undef EXPENSIVE_CHECK +#define EXPENSIVE_CHECK(x) ASSERT_DIE(x) +#endif #else #define ASSERT(x) do { if (!(x)) log(L_BUG "Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) #define ASSUME(x) /* intentionally left blank */ #endif + #ifdef DEBUGGING asm( ".pushsection \".debug_gdb_scripts\", \"MS\",@progbits,1\n" From baac7009063d94799821422ecc63ea2af41561ea Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 14 Aug 2019 16:23:58 +0200 Subject: [PATCH 10/26] List expensive check. --- lib/lists.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/lib/lists.c b/lib/lists.c index 4a48d3b7..c162a991 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -29,6 +29,42 @@ #include "nest/bird.h" #include "lib/lists.h" +LIST_INLINE int +check_list(list *l, node *n) +{ + if (!l) + { + ASSERT_DIE(n); + ASSERT_DIE(n->prev); + + do { n = n->prev; } while (n->prev); + + l = SKIP_BACK(list, head_node, n); + } + + int seen = 0; + + ASSERT_DIE(l->null == NULL); + ASSERT_DIE(l->head != NULL); + ASSERT_DIE(l->tail != NULL); + + node *prev = &l->head_node, *cur = l->head, *next = l->head->next; + while (next) + { + if (cur == n) + seen++; + ASSERT_DIE(cur->prev == prev); + prev = cur; + cur = next; + next = next->next; + } + + ASSERT_DIE(cur == &(l->tail_node)); + ASSERT_DIE(!n || (seen == 1)); + + return 1; +} + /** * add_tail - append a node to a list * @l: linked list @@ -39,6 +75,10 @@ LIST_INLINE void add_tail(list *l, node *n) { + EXPENSIVE_CHECK(check_list(l, NULL)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = l->tail; n->next = &l->tail_node; @@ -57,6 +97,10 @@ add_tail(list *l, node *n) LIST_INLINE void add_head(list *l, node *n) { + EXPENSIVE_CHECK(check_list(l, NULL)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = l->head; n->next = z; @@ -76,6 +120,10 @@ add_head(list *l, node *n) LIST_INLINE void insert_node(node *n, node *after) { + EXPENSIVE_CHECK(check_list(l, after)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = after->next; n->next = z; @@ -93,6 +141,8 @@ insert_node(node *n, node *after) LIST_INLINE void rem_node(node *n) { + EXPENSIVE_CHECK(check_list(NULL, n)); + node *z = n->prev; node *x = n->next; @@ -116,6 +166,10 @@ rem_node(node *n) LIST_INLINE void replace_node(node *old, node *new) { + EXPENSIVE_CHECK(check_list(NULL, old)); + ASSUME(new->prev == NULL); + ASSUME(new->next == NULL); + old->next->prev = new; old->prev->next = new; @@ -149,6 +203,9 @@ init_list(list *l) LIST_INLINE void add_tail_list(list *to, list *l) { + EXPENSIVE_CHECK(check_list(to, NULL)); + EXPENSIVE_CHECK(check_list(l, NULL)); + node *p = to->tail; node *q = l->head; @@ -157,6 +214,8 @@ add_tail_list(list *to, list *l) q = l->tail; q->next = &to->tail_node; to->tail = q; + + EXPENSIVE_CHECK(check_list(to, NULL)); } LIST_INLINE uint @@ -165,6 +224,8 @@ list_length(list *l) uint len = 0; node *n; + EXPENSIVE_CHECK(check_list(l, NULL)); + WALK_LIST(n, *l) len++; From dccee408262262ab9a63403141b74a0d937284bc Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 16 Aug 2019 12:47:13 +0200 Subject: [PATCH 11/26] OSPF: variable-length array of size 0 replaced by alloca()'d pointer NULL pointer is safer than a random pointer onto stack if this function gets changed and eventually broken. --- proto/ospf/ospf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c index 29610f4a..3cb40283 100644 --- a/proto/ospf/ospf.c +++ b/proto/ospf/ospf.c @@ -1244,7 +1244,7 @@ ospf_sh_state(struct proto *P, int verbose, int reachable) uint num = p->gr->hash_entries; struct top_hash_entry *hea[num]; - struct top_hash_entry *hex[verbose ? num : 0]; + struct top_hash_entry **hex = verbose ? alloca(num * sizeof(struct top_hash_entry *)) : NULL; struct top_hash_entry *he; struct top_hash_entry *cnode = NULL; @@ -1289,7 +1289,9 @@ ospf_sh_state(struct proto *P, int verbose, int reachable) lsa_compare_ospf3 = !ospf2; qsort(hea, j1, sizeof(struct top_hash_entry *), lsa_compare_for_state); - qsort(hex, jx, sizeof(struct top_hash_entry *), ext_compare_for_state); + + if (verbose) + qsort(hex, jx, sizeof(struct top_hash_entry *), ext_compare_for_state); /* * This code is a bit tricky, we have a primary LSAs (router and From 9e64ac4b7c23aa3b8b9149794c05305315cf31e5 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 16 Aug 2019 14:04:53 +0200 Subject: [PATCH 12/26] OSPF: Adding a note about a static analyzer result. --- proto/ospf/topology.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/proto/ospf/topology.c b/proto/ospf/topology.c index 2e9c3965..c8ec730a 100644 --- a/proto/ospf/topology.c +++ b/proto/ospf/topology.c @@ -329,6 +329,14 @@ ospf_originate_lsa(struct ospf_proto *p, struct ospf_new_lsa *lsa) en->next_lsa_opts = 0; } + /* The static analyzer complains here that en->lsa_body may be NULL. + * Yes, it may if ospf_hash_get() creates a new struct top_hash_entry. + * In this case, also en->lsa.length must be 0 and lsa_length is never + * equal to 0 while sizeof(struct ospf_lsa_header) is non-zero. + * Therefore memcmp() is never executed with NULL here. + * */ + ASSUME((en->lsa.length == 0) == (en->lsa_body == NULL)); + /* Ignore the the new LSA if is the same as the current one */ if ((en->lsa.age < LSA_MAXAGE) && (lsa_length == en->lsa.length) && From b7482209065a03c3186d74e5e4129539ce7a3ce4 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 16 Aug 2019 21:15:49 +0200 Subject: [PATCH 13/26] Static check: Don't report dead code --- Makefile.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index cf94e352..2698e34d 100644 --- a/Makefile.in +++ b/Makefile.in @@ -184,8 +184,9 @@ check: tests tests_run tests: $(tests_targets) tests_run: $(tests_targets_ok) -STATIC_CHECKERS := nullability.NullableDereferenced nullability.NullablePassedToNonnull nullability.NullableReturnedFromNonnull optin.portability.UnixAPI valist.CopyToSelf valist.Uninitialized valist.Unterminated -STATIC_SCAN_FLAGS := --force-analyze-debug-code -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS)) +STATIC_CHECKERS_ENABLE := nullability.NullableDereferenced nullability.NullablePassedToNonnull nullability.NullableReturnedFromNonnull optin.portability.UnixAPI valist.CopyToSelf valist.Uninitialized valist.Unterminated +STATIC_CHECKERS_DISABLE := deadcode.DeadStores +STATIC_SCAN_FLAGS := --force-analyze-debug-code -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS_ENABLE)) $(addprefix -disable-checker ,$(STATIC_CHECKERS_DISABLE)) static-scan: $(E)echo Running static code analysis From 5f60d14edeb48824d28e6393e7eb1aa50d5f2cd1 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 16 Aug 2019 21:22:56 +0200 Subject: [PATCH 14/26] RPKI: fixed rare va_list leak --- proto/rpki/packets.c | 1 + 1 file changed, 1 insertion(+) diff --git a/proto/rpki/packets.c b/proto/rpki/packets.c index 59a5efaf..e9d24fb8 100644 --- a/proto/rpki/packets.c +++ b/proto/rpki/packets.c @@ -1011,6 +1011,7 @@ rpki_send_error_pdu(struct rpki_cache *cache, const enum pdu_error_type error_co { va_start(args, fmt); msg_len = bvsnprintf(msg, sizeof(msg), fmt, args) + 1; + va_end(args); } u32 pdu_size = 16 + err_pdu_len + msg_len; From a08853a26989d343c507a41257dedcdea3befd73 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 08:54:08 +0200 Subject: [PATCH 15/26] Static scanner and expensive debugging setup fix --- Makefile.in | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index 2698e34d..da6cd206 100644 --- a/Makefile.in +++ b/Makefile.in @@ -186,7 +186,7 @@ tests_run: $(tests_targets_ok) STATIC_CHECKERS_ENABLE := nullability.NullableDereferenced nullability.NullablePassedToNonnull nullability.NullableReturnedFromNonnull optin.portability.UnixAPI valist.CopyToSelf valist.Uninitialized valist.Unterminated STATIC_CHECKERS_DISABLE := deadcode.DeadStores -STATIC_SCAN_FLAGS := --force-analyze-debug-code -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS_ENABLE)) $(addprefix -disable-checker ,$(STATIC_CHECKERS_DISABLE)) +STATIC_SCAN_FLAGS := -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS_ENABLE)) $(addprefix -disable-checker ,$(STATIC_CHECKERS_DISABLE)) static-scan: $(E)echo Running static code analysis diff --git a/configure.ac b/configure.ac index 96f66644..eabb3d56 100644 --- a/configure.ac +++ b/configure.ac @@ -79,7 +79,7 @@ AC_ARG_VAR([BISON], [location of the Bison program]) AC_ARG_VAR([M4], [location of the M4 program]) if test "$enable_debug_expensive" = yes; then - enable_debug = yes + enable_debug=yes fi if test "$srcdir" = . ; then From bbe49ae569534d0cf7ff226d19e729dcc764e606 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 08:59:06 +0200 Subject: [PATCH 16/26] Nest: Assumption in rt-show for not-so-intuitive invariant. --- nest/rt-show.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nest/rt-show.c b/nest/rt-show.c index 3431293a..bd0df9ee 100644 --- a/nest/rt-show.c +++ b/nest/rt-show.c @@ -104,6 +104,12 @@ rt_show_net(struct cli *c, net *n, struct rt_show_data *d) rte *e, *ee; byte ia[NET_MAX_TEXT_LENGTH+1]; struct channel *ec = d->tab->export_channel; + + /* The Clang static analyzer complains that ec may be NULL. + * It should be ensured to be not NULL by rt_show_prepare_tables() */ + if (d->export_mode) + ASSUME(ec); + int first = 1; int pass = 0; From 0fa8bf91cd474e393ded85b329fde30ba35f6a26 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 10:20:46 +0200 Subject: [PATCH 17/26] Nest: Several assumptions to tame the static analyzer --- nest/rt-attr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nest/rt-attr.c b/nest/rt-attr.c index 8620d321..3f83edce 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -202,7 +202,7 @@ nexthop__same(struct nexthop *x, struct nexthop *y) } static int -nexthop_compare_node(const struct nexthop *x, const struct nexthop *y) +nexthop_compare_node(const struct nexthop *x, const struct nexthop *y) { int r; @@ -278,18 +278,22 @@ nexthop_merge(struct nexthop *x, struct nexthop *y, int rx, int ry, int max, lin while ((x || y) && max--) { int cmp = nexthop_compare_node(x, y); + if (cmp < 0) { + ASSUME(x); *n = rx ? x : nexthop_copy_node(x, lp); x = x->next; } else if (cmp > 0) { + ASSUME(y); *n = ry ? y : nexthop_copy_node(y, lp); y = y->next; } else { + ASSUME(x && y); *n = rx ? x : (ry ? y : nexthop_copy_node(x, lp)); x = x->next; y = y->next; From a7d9b8f116d00194e94c7505cbc8ed7f8784eeab Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 10:28:55 +0200 Subject: [PATCH 18/26] OSPF: Zero-initialization of a temporary neighbor --- proto/ospf/neighbor.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index 30e80640..18692d6e 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -650,19 +650,20 @@ void ospf_dr_election(struct ospf_iface *ifa) { struct ospf_proto *p = ifa->oa->po; - struct ospf_neighbor *neigh, *ndr, *nbdr, me; + struct ospf_neighbor *neigh, *ndr, *nbdr; u32 myid = p->router_id; DBG("(B)DR election.\n"); - me.state = NEIGHBOR_2WAY; - me.rid = myid; - me.priority = ifa->priority; - me.ip = ifa->addr->ip; - - me.dr = ospf_is_v2(p) ? ipa_to_u32(ifa->drip) : ifa->drid; - me.bdr = ospf_is_v2(p) ? ipa_to_u32(ifa->bdrip) : ifa->bdrid; - me.iface_id = ifa->iface_id; + struct ospf_neighbor me = { + .state = NEIGHBOR_2WAY, + .rid = myid, + .priority = ifa->priority, + .ip = ifa->addr->ip, + .dr = ospf_is_v2(p) ? ipa_to_u32(ifa->drip) : ifa->drid, + .bdr = ospf_is_v2(p) ? ipa_to_u32(ifa->bdrip) : ifa->bdrid, + .iface_id = ifa->iface_id, + }; add_tail(&ifa->neigh_list, NODE & me); From 258be56539a3d4b47fe779f9658ca3d88761878d Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 13:36:36 +0200 Subject: [PATCH 19/26] Nest: Added const to ea_show just to declare that this shouldn't really change anything --- nest/protocol.h | 2 +- nest/route.h | 2 +- nest/rt-attr.c | 6 +++--- proto/babel/babel.c | 2 +- proto/bgp/attrs.c | 16 ++++++++-------- proto/bgp/bgp.h | 2 +- proto/ospf/ospf.c | 2 +- proto/radv/radv.c | 2 +- proto/rip/rip.c | 2 +- sysdep/bsd/krt-sys.h | 2 +- sysdep/linux/netlink.c | 2 +- sysdep/unix/krt.c | 2 +- sysdep/unix/krt.h | 2 +- 13 files changed, 22 insertions(+), 22 deletions(-) diff --git a/nest/protocol.h b/nest/protocol.h index e97e59dd..a934c047 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -80,7 +80,7 @@ struct protocol { void (*cleanup)(struct proto *); /* Called after shutdown when protocol became hungry/down */ void (*get_status)(struct proto *, byte *buf); /* Get instance status (for `show protocols' command) */ void (*get_route_info)(struct rte *, byte *buf); /* Get route information (for `show route' command) */ - int (*get_attr)(struct eattr *, byte *buf, int buflen); /* ASCIIfy dynamic attribute (returns GA_*) */ + int (*get_attr)(const struct eattr *, byte *buf, int buflen); /* ASCIIfy dynamic attribute (returns GA_*) */ void (*show_proto_info)(struct proto *); /* Show protocol info (for `show protocols all' command) */ void (*copy_config)(struct proto_config *, struct proto_config *); /* Copy config from given protocol instance */ }; diff --git a/nest/route.h b/nest/route.h index 5421ece5..1b4f2866 100644 --- a/nest/route.h +++ b/nest/route.h @@ -577,7 +577,7 @@ void ea_merge(ea_list *from, ea_list *to); /* Merge sub-lists to allocated buffe int ea_same(ea_list *x, ea_list *y); /* Test whether two ea_lists are identical */ uint ea_hash(ea_list *e); /* Calculate 16-bit hash value */ ea_list *ea_append(ea_list *to, ea_list *what); -void ea_format_bitfield(struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max); +void ea_format_bitfield(const struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max); #define ea_normalize(ea) do { \ if (ea->next) { \ diff --git a/nest/rt-attr.c b/nest/rt-attr.c index 3f83edce..28d956bc 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -790,7 +790,7 @@ ea_free(ea_list *o) } static int -get_generic_attr(eattr *a, byte **buf, int buflen UNUSED) +get_generic_attr(const eattr *a, byte **buf, int buflen UNUSED) { if (a->id == EA_GEN_IGP_METRIC) { @@ -802,7 +802,7 @@ get_generic_attr(eattr *a, byte **buf, int buflen UNUSED) } void -ea_format_bitfield(struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max) +ea_format_bitfield(const struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max) { byte *bound = buf + bufsize - 32; u32 data = a->u.data; @@ -898,7 +898,7 @@ ea_show_lc_set(struct cli *c, const struct adata *ad, byte *pos, byte *buf, byte * get_attr() hook, it's consulted first. */ void -ea_show(struct cli *c, eattr *e) +ea_show(struct cli *c, const eattr *e) { struct protocol *p; int status = GA_UNKNOWN; diff --git a/proto/babel/babel.c b/proto/babel/babel.c index a915e8fa..ebd5f7cc 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1852,7 +1852,7 @@ babel_get_route_info(rte *rte, byte *buf) } static int -babel_get_attr(eattr *a, byte *buf, int buflen UNUSED) +babel_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 655ddb62..4710bfba 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -72,7 +72,7 @@ struct bgp_attr_desc { void (*export)(struct bgp_export_state *s, eattr *a); int (*encode)(struct bgp_write_state *s, eattr *a, byte *buf, uint size); void (*decode)(struct bgp_parse_state *s, uint code, uint flags, byte *data, uint len, ea_list **to); - void (*format)(eattr *ea, byte *buf, uint size); + void (*format)(const eattr *ea, byte *buf, uint size); }; static const struct bgp_attr_desc bgp_attr_table[]; @@ -396,7 +396,7 @@ bgp_decode_origin(struct bgp_parse_state *s, uint code UNUSED, uint flags, byte } static void -bgp_format_origin(eattr *a, byte *buf, uint size UNUSED) +bgp_format_origin(const eattr *a, byte *buf, uint size UNUSED) { static const char *bgp_origin_names[] = { "IGP", "EGP", "Incomplete" }; @@ -510,7 +510,7 @@ bgp_decode_next_hop(struct bgp_parse_state *s, uint code UNUSED, uint flags UNUS /* TODO: This function should use AF-specific hook */ static void -bgp_format_next_hop(eattr *a, byte *buf, uint size UNUSED) +bgp_format_next_hop(const eattr *a, byte *buf, uint size UNUSED) { ip_addr *nh = (void *) a->u.ptr->data; uint len = a->u.ptr->length; @@ -601,7 +601,7 @@ bgp_decode_aggregator(struct bgp_parse_state *s, uint code UNUSED, uint flags, b } static void -bgp_format_aggregator(eattr *a, byte *buf, uint size UNUSED) +bgp_format_aggregator(const eattr *a, byte *buf, uint size UNUSED) { const byte *data = a->u.ptr->data; @@ -676,7 +676,7 @@ bgp_decode_cluster_list(struct bgp_parse_state *s, uint code UNUSED, uint flags, } static void -bgp_format_cluster_list(eattr *a, byte *buf, uint size) +bgp_format_cluster_list(const eattr *a, byte *buf, uint size) { /* Truncates cluster lists larger than buflen, probably not a problem */ int_set_format(a->u.ptr, 0, -1, buf, size); @@ -831,7 +831,7 @@ bgp_decode_aigp(struct bgp_parse_state *s, uint code UNUSED, uint flags, byte *d } static void -bgp_format_aigp(eattr *a, byte *buf, uint size UNUSED) +bgp_format_aigp(const eattr *a, byte *buf, uint size UNUSED) { const byte *b = bgp_aigp_get_tlv(a->u.ptr, BGP_AIGP_METRIC); @@ -909,7 +909,7 @@ bgp_decode_mpls_label_stack(struct bgp_parse_state *s, uint code UNUSED, uint fl } static void -bgp_format_mpls_label_stack(eattr *a, byte *buf, uint size) +bgp_format_mpls_label_stack(const eattr *a, byte *buf, uint size) { u32 *labels = (u32 *) a->u.ptr->data; uint lnum = a->u.ptr->length / 4; @@ -2293,7 +2293,7 @@ bgp_process_as4_attrs(ea_list **attrs, struct linpool *pool) } int -bgp_get_attr(eattr *a, byte *buf, int buflen) +bgp_get_attr(const eattr *a, byte *buf, int buflen) { uint i = EA_ID(a->id); const struct bgp_attr_desc *d; diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index 0529c45a..455f04f9 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -581,7 +581,7 @@ int bgp_rte_recalculate(rtable *table, net *net, rte *new, rte *old, rte *old_be struct rte *bgp_rte_modify_stale(struct rte *r, struct linpool *pool); void bgp_rt_notify(struct proto *P, struct channel *C, net *n, rte *new, rte *old); int bgp_preexport(struct proto *, struct rte **, struct linpool *); -int bgp_get_attr(struct eattr *e, byte *buf, int buflen); +int bgp_get_attr(const struct eattr *e, byte *buf, int buflen); void bgp_get_route_info(struct rte *, byte *buf); int bgp_total_aigp_metric_(rte *e, u64 *metric, const struct adata **ad); diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c index 3cb40283..c8ed0e06 100644 --- a/proto/ospf/ospf.c +++ b/proto/ospf/ospf.c @@ -620,7 +620,7 @@ ospf_get_route_info(rte * rte, byte * buf) } static int -ospf_get_attr(eattr * a, byte * buf, int buflen UNUSED) +ospf_get_attr(const eattr * a, byte * buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/radv/radv.c b/proto/radv/radv.c index 622b3c3c..b4235917 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -740,7 +740,7 @@ radv_pref_str(u32 pref) /* The buffer has some minimal size */ static int -radv_get_attr(eattr *a, byte *buf, int buflen UNUSED) +radv_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/rip/rip.c b/proto/rip/rip.c index f02d5071..ae8007d9 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -1190,7 +1190,7 @@ rip_get_route_info(rte *rte, byte *buf) } static int -rip_get_attr(eattr *a, byte *buf, int buflen UNUSED) +rip_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/sysdep/bsd/krt-sys.h b/sysdep/bsd/krt-sys.h index aa6cc72e..57501884 100644 --- a/sysdep/bsd/krt-sys.h +++ b/sysdep/bsd/krt-sys.h @@ -46,7 +46,7 @@ static inline void krt_sys_io_init(void) { } static inline void krt_sys_init(struct krt_proto *p UNUSED) { } static inline void krt_sys_postconfig(struct krt_config *x UNUSED) { } -static inline int krt_sys_get_attr(eattr *a UNUSED, byte *buf UNUSED, int buflen UNUSED) { return GA_UNKNOWN; } +static inline int krt_sys_get_attr(const eattr *a UNUSED, byte *buf UNUSED, int buflen UNUSED) { return GA_UNKNOWN; } #endif diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index 856869ac..a9e711b4 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -2065,7 +2065,7 @@ static const char *krt_features_names[KRT_FEATURES_MAX] = { }; int -krt_sys_get_attr(eattr *a, byte *buf, int buflen UNUSED) +krt_sys_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index 42dd12f6..cccee456 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -1156,7 +1156,7 @@ krt_copy_config(struct proto_config *dest, struct proto_config *src) } static int -krt_get_attr(eattr *a, byte *buf, int buflen) +krt_get_attr(const eattr *a, byte *buf, int buflen) { switch (a->id) { diff --git a/sysdep/unix/krt.h b/sysdep/unix/krt.h index 6066f2f1..4a5d10d2 100644 --- a/sysdep/unix/krt.h +++ b/sysdep/unix/krt.h @@ -141,7 +141,7 @@ void krt_sys_copy_config(struct krt_config *, struct krt_config *); int krt_capable(rte *e); void krt_do_scan(struct krt_proto *); void krt_replace_rte(struct krt_proto *p, net *n, rte *new, rte *old); -int krt_sys_get_attr(eattr *a, byte *buf, int buflen); +int krt_sys_get_attr(const eattr *a, byte *buf, int buflen); /* kif sysdep */ From 3bb10b4d31d68a8139e284c27f7eb6fca897721d Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 14:18:41 +0200 Subject: [PATCH 20/26] Uninitialized list nodes fixes --- lib/resource.c | 1 + nest/rt-table.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/resource.c b/lib/resource.c index ab8c800f..7e624321 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -340,6 +340,7 @@ mb_alloc(pool *p, unsigned size) struct mblock *b = xmalloc(sizeof(struct mblock) + size); b->r.class = &mb_class; + b->r.n = (node) {}; add_tail(&p->inside, &b->r.n); b->size = size; return b->data; diff --git a/nest/rt-table.c b/nest/rt-table.c index a46eeb77..ae5a8444 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2304,7 +2304,7 @@ rt_commit(struct config *new, struct config *old) WALK_LIST(r, new->tables) if (!r->table) { - rtable *t = mb_alloc(rt_table_pool, sizeof(struct rtable)); + rtable *t = mb_allocz(rt_table_pool, sizeof(struct rtable)); DBG("\t%s: created\n", r->name); rt_setup(rt_table_pool, t, r); add_tail(&routing_tables, &t->n); From e26a5195dd6c62e6f4b80d13b6ecd5f40ee68546 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 14:03:47 +0200 Subject: [PATCH 21/26] Lists: fix a stupid sanitizer bug --- lib/lists.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/lists.c b/lib/lists.c index c162a991..8553ee27 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -167,8 +167,12 @@ LIST_INLINE void replace_node(node *old, node *new) { EXPENSIVE_CHECK(check_list(NULL, old)); - ASSUME(new->prev == NULL); - ASSUME(new->next == NULL); + + if (old != new) + { + ASSUME(new->prev == NULL); + ASSUME(new->next == NULL); + } old->next->prev = new; old->prev->next = new; From 9ac13d7af25d6b26866bf4f4a4fab371925fb1df Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Mon, 19 Aug 2019 14:36:51 +0200 Subject: [PATCH 22/26] Lists: Replaced replace_node() by update_node() which is the only use of that function. --- lib/lists.c | 28 ++++++++-------------------- lib/lists_test.c | 13 ++++++++----- lib/resource.c | 2 +- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/lib/lists.c b/lib/lists.c index 8553ee27..200576cf 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -153,32 +153,20 @@ rem_node(node *n) } /** - * replace_node - replace a node in a list with another one - * @old: node to be removed - * @new: node to be inserted + * update_node - update node after calling realloc on it + * @n: node to be updated * - * Replaces node @old in the list it's linked in with node @new. Node - * @old may be a copy of the original node, which is not accessed - * through the list. The function could be called with @old == @new, - * which just fixes neighbors' pointers in the case that the node - * was reallocated. + * Fixes neighbor pointers. */ LIST_INLINE void -replace_node(node *old, node *new) +update_node(node *n) { - EXPENSIVE_CHECK(check_list(NULL, old)); + ASSUME(n->next->prev == n->prev->next); - if (old != new) - { - ASSUME(new->prev == NULL); - ASSUME(new->next == NULL); - } + n->next->prev = n; + n->prev->next = n; - old->next->prev = new; - old->prev->next = new; - - new->prev = old->prev; - new->next = old->next; + EXPENSIVE_CHECK(check_list(NULL, n)); } /** diff --git a/lib/lists_test.c b/lib/lists_test.c index f26a88e2..cf0021fe 100644 --- a/lib/lists_test.c +++ b/lib/lists_test.c @@ -222,26 +222,29 @@ t_remove_node(void) } static int -t_replace_node(void) +t_update_node(void) { node head, inside, tail; init_list_(); fill_list(); - replace_node(&nodes[0], &head); + head = nodes[0]; + update_node(&head); bt_assert(l.head == &head); bt_assert(head.prev == NODE &l.head); bt_assert(head.next == &nodes[1]); bt_assert(nodes[1].prev == &head); - replace_node(&nodes[MAX_NUM/2], &inside); + inside = nodes[MAX_NUM/2]; + update_node(&inside); bt_assert(nodes[MAX_NUM/2-1].next == &inside); bt_assert(nodes[MAX_NUM/2+1].prev == &inside); bt_assert(inside.prev == &nodes[MAX_NUM/2-1]); bt_assert(inside.next == &nodes[MAX_NUM/2+1]); - replace_node(&nodes[MAX_NUM-1], &tail); + tail = nodes[MAX_NUM-1]; + update_node(&tail); bt_assert(l.tail == &tail); bt_assert(tail.prev == &nodes[MAX_NUM-2]); bt_assert(tail.next == NODE &l.null); @@ -280,7 +283,7 @@ main(int argc, char *argv[]) bt_test_suite(t_add_head, "Adding nodes to head of list"); bt_test_suite(t_insert_node, "Inserting nodes to list"); bt_test_suite(t_remove_node, "Removing nodes from list"); - bt_test_suite(t_replace_node, "Replacing nodes in list"); + bt_test_suite(t_update_node, "Updating nodes in list"); bt_test_suite(t_add_tail_list, "At the tail of a list adding the another list"); return bt_exit_value(); diff --git a/lib/resource.c b/lib/resource.c index 7e624321..5589373e 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -388,7 +388,7 @@ mb_realloc(void *m, unsigned size) struct mblock *b = SKIP_BACK(struct mblock, data, m); b = xrealloc(b, sizeof(struct mblock) + size); - replace_node(&b->r.n, &b->r.n); + update_node(&b->r.n); b->size = size; return b->data; } From cdde3550dc188f493daf82ef9d9acf8b85d9d722 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 14:57:41 +0200 Subject: [PATCH 23/26] Unix socket: Path length check directly before copying the path. This is not needed as the string is always short enough, anyway it may be needed in future and one strlen during BIRD start is cheap enough. --- sysdep/unix/io.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index f6cc0e32..9d54a2c3 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -1495,7 +1495,9 @@ sk_open_unix(sock *s, char *name) if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) return -1; - /* Path length checked in test_old_bird() */ + /* Path length checked in test_old_bird() but we may need unix sockets for other reasons in future */ + ASSERT_DIE(strlen(name) < sizeof(sa.sun_path)); + sa.sun_family = AF_UNIX; strcpy(sa.sun_path, name); From 0c3b8ffe25f43e59dfe8b1aeb219ff1cb4c6aa2b Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 15:03:09 +0200 Subject: [PATCH 24/26] Lexer: strtoul shall never set endptr to NULL; it should be an error --- conf/cf-lex.l | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/conf/cf-lex.l b/conf/cf-lex.l index 1d6cae2c..9ea05e9d 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -169,7 +169,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(yytext, &e); - if (e && (*e != ':') || (errno == ERANGE) || (l >> 32)) + if (!e || (*e != ':') || (errno == ERANGE) || (l >> 32)) cf_error("ASN out of range"); if (l >> 16) @@ -187,7 +187,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(e+1, &e); - if (e && *e || (errno == ERANGE) || (l >> len2)) + if (!e || *e || (errno == ERANGE) || (l >> len2)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -214,13 +214,13 @@ WHITE [ \t] errno = 0; l = bstrtoul10(yytext+2, &e); - if (e && (*e != ':') || (errno == ERANGE) || (l >> len1)) + if (!e || (*e != ':') || (errno == ERANGE) || (l >> len1)) cf_error("ASN out of range"); cf_lval.i64 |= ((u64) l) << len2; errno = 0; l = bstrtoul10(e+1, &e); - if (e && *e || (errno == ERANGE) || (l >> len2)) + if (!e || *e || (errno == ERANGE) || (l >> len2)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -242,7 +242,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(e, &e); - if (e && *e || (errno == ERANGE) || (l >> 16)) + if (!e || *e || (errno == ERANGE) || (l >> 16)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -266,7 +266,7 @@ WHITE [ \t] unsigned long int l; errno = 0; l = bstrtoul16(yytext+2, &e); - if (e && *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) + if (!e || *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) cf_error("Number out of range"); cf_lval.i = l; return NUM; @@ -277,7 +277,7 @@ WHITE [ \t] unsigned long int l; errno = 0; l = bstrtoul10(yytext, &e); - if (e && *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) + if (!e || *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) cf_error("Number out of range"); cf_lval.i = l; return NUM; From ea259d6201973eb0f764cbb2bb6549b6ac79b316 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 17 Aug 2019 16:09:29 +0200 Subject: [PATCH 25/26] Timer: Adding missing initializer. --- lib/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timer.c b/lib/timer.c index be695114..a5abbcc4 100644 --- a/lib/timer.c +++ b/lib/timer.c @@ -255,7 +255,7 @@ timer_init(void) btime tm_parse_time(const char *x) { - struct tm tm; + struct tm tm = {}; int usec, n1, n2, n3, r; r = sscanf(x, "%d-%d-%d%n %d:%d:%d%n.%d%n", From 59238768b3b05fa134348d2232b42537d0403994 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Mon, 19 Aug 2019 14:43:14 +0200 Subject: [PATCH 26/26] Slab: Init node in slab head to NULLs. --- lib/slab.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/slab.c b/lib/slab.c index 5c414f9e..c3035b45 100644 --- a/lib/slab.c +++ b/lib/slab.c @@ -216,8 +216,11 @@ sl_new_head(slab *s) struct sl_obj *no; uint n = s->objs_per_slab; - h->first_free = o; - h->num_full = 0; + *h = (struct sl_head) { + .first_free = o, + .num_full = 0, + }; + while (n--) { o->slab = h;