From 31b2cc24c16db0b80f3096c749f0acd6b9643578 Mon Sep 17 00:00:00 2001 From: Michal Ruprich Date: Tue, 20 Jul 2021 09:35:21 +0200 Subject: [PATCH] Resolves: #1983278 - ospfd crashes in route_node_delete with assertion fail --- 0007-ospfd-crash.patch | 108 +++++++++++++++++++++++++++++++++++++++++ frr.spec | 6 ++- 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 0007-ospfd-crash.patch diff --git a/0007-ospfd-crash.patch b/0007-ospfd-crash.patch new file mode 100644 index 0000000..2eceab9 --- /dev/null +++ b/0007-ospfd-crash.patch @@ -0,0 +1,108 @@ +From 4f08c715db6893ff439d0a39bf4506cd26256d13 Mon Sep 17 00:00:00 2001 +From: Igor Ryzhov +Date: Fri, 18 Jun 2021 13:06:13 +0300 +Subject: [PATCH] lib: remove pure attribute from functions that modify memory + +Almost all functions currently marked with pure attribute acquire a +route_node lock. By marking them pure we allow compiler to optimize the +code and not call them when it already knows the return value. This is +completely incorrect. + +Only two of eleven functions can be marked as pure. And they still won't +be optimized because they are never called from the same function twice. +Let's remove the ext_pure macro completely to reduce the chance of +repeating this mistake in the future. + +Fixes #8866, #8809, #8595, #6992. + +Signed-off-by: Igor Ryzhov +--- + lib/compiler.h | 9 --------- + lib/table.h | 44 ++++++++++++++++++++------------------------ + 2 files changed, 20 insertions(+), 33 deletions(-) + +diff --git a/lib/compiler.h b/lib/compiler.h +index bbfe01b569c..e805eb8be48 100644 +--- a/lib/compiler.h ++++ b/lib/compiler.h +@@ -123,15 +123,6 @@ extern "C" { + #define assume(x) + #endif + +-/* pure = function does not modify memory & return value is the same if +- * memory hasn't changed (=> allows compiler to optimize) +- * +- * Mostly autodetected by the compiler if function body is available (i.e. +- * static inline functions in headers). Since that implies it should only be +- * used in headers for non-inline functions, the "extern" is included here. +- */ +-#define ext_pure extern __attribute__((pure)) +- + /* for helper functions defined inside macros */ + #define macro_inline static inline __attribute__((unused)) + #define macro_pure static inline __attribute__((unused, pure)) +diff --git a/lib/table.h b/lib/table.h +index 7e383dce808..5dec69ee7ea 100644 +--- a/lib/table.h ++++ b/lib/table.h +@@ -197,29 +197,25 @@ static inline void route_table_set_info(struct route_table *table, void *d) + table->info = d; + } + +-/* ext_pure => extern __attribute__((pure)) +- * does not modify memory (but depends on mem), allows compiler to optimize +- */ +- + extern void route_table_finish(struct route_table *table); +-ext_pure struct route_node *route_top(struct route_table *table); +-ext_pure struct route_node *route_next(struct route_node *node); +-ext_pure struct route_node *route_next_until(struct route_node *node, +- const struct route_node *limit); ++extern struct route_node *route_top(struct route_table *table); ++extern struct route_node *route_next(struct route_node *node); ++extern struct route_node *route_next_until(struct route_node *node, ++ const struct route_node *limit); + extern struct route_node *route_node_get(struct route_table *table, + union prefixconstptr pu); +-ext_pure struct route_node *route_node_lookup(struct route_table *table, +- union prefixconstptr pu); +-ext_pure struct route_node *route_node_lookup_maynull(struct route_table *table, +- union prefixconstptr pu); +-ext_pure struct route_node *route_node_match(struct route_table *table, +- union prefixconstptr pu); +-ext_pure struct route_node *route_node_match_ipv4(struct route_table *table, +- const struct in_addr *addr); +-ext_pure struct route_node *route_node_match_ipv6(struct route_table *table, +- const struct in6_addr *addr); +- +-ext_pure unsigned long route_table_count(struct route_table *table); ++extern struct route_node *route_node_lookup(struct route_table *table, ++ union prefixconstptr pu); ++extern struct route_node *route_node_lookup_maynull(struct route_table *table, ++ union prefixconstptr pu); ++extern struct route_node *route_node_match(struct route_table *table, ++ union prefixconstptr pu); ++extern struct route_node *route_node_match_ipv4(struct route_table *table, ++ const struct in_addr *addr); ++extern struct route_node *route_node_match_ipv6(struct route_table *table, ++ const struct in6_addr *addr); ++ ++extern unsigned long route_table_count(struct route_table *table); + + extern struct route_node *route_node_create(route_table_delegate_t *delegate, + struct route_table *table); +@@ -228,10 +224,10 @@ extern void route_node_destroy(route_table_delegate_t *delegate, + struct route_table *table, + struct route_node *node); + +-ext_pure struct route_node *route_table_get_next(struct route_table *table, +- union prefixconstptr pu); +-ext_pure int route_table_prefix_iter_cmp(const struct prefix *p1, +- const struct prefix *p2); ++extern struct route_node *route_table_get_next(struct route_table *table, ++ union prefixconstptr pu); ++extern int route_table_prefix_iter_cmp(const struct prefix *p1, ++ const struct prefix *p2); + + /* + * Iterator functions. diff --git a/frr.spec b/frr.spec index bf84e15..755625f 100644 --- a/frr.spec +++ b/frr.spec @@ -5,7 +5,7 @@ Name: frr Version: 7.5.1 -Release: 6%{?dist} +Release: 7%{?dist} Summary: Routing daemon License: GPLv2+ URL: http://www.frrouting.org @@ -20,6 +20,7 @@ Patch0003: 0003-disable-eigrp-crypto.patch Patch0004: 0004-fips-mode.patch Patch0005: 0005-icc-options.patch Patch0006: 0006-move-to-libexec.patch +Patch0007: 0007-ospfd-crash.patch BuildRequires: autoconf BuildRequires: automake @@ -203,6 +204,9 @@ fi %{_sysusersdir}/%{name}.conf %changelog +* Tue Jul 20 2021 Michal Ruprich - 7.5.1-7 +- Resolves: #1983278 - ospfd crashes in route_node_delete with assertion fail + * Sat Jul 10 2021 Björn Esser - 7.5.1-6 - Rebuild for versioned symbols in json-c