Resolves: #1983278 - ospfd crashes in route_node_delete with assertion fail

This commit is contained in:
Michal Ruprich 2021-07-20 09:35:21 +02:00
parent 60a5f4164e
commit 31b2cc24c1
2 changed files with 113 additions and 1 deletions

108
0007-ospfd-crash.patch Normal file
View File

@ -0,0 +1,108 @@
From 4f08c715db6893ff439d0a39bf4506cd26256d13 Mon Sep 17 00:00:00 2001
From: Igor Ryzhov <iryzhov@nfware.com>
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 <iryzhov@nfware.com>
---
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.

View File

@ -5,7 +5,7 @@
Name: frr Name: frr
Version: 7.5.1 Version: 7.5.1
Release: 6%{?dist} Release: 7%{?dist}
Summary: Routing daemon Summary: Routing daemon
License: GPLv2+ License: GPLv2+
URL: http://www.frrouting.org URL: http://www.frrouting.org
@ -20,6 +20,7 @@ Patch0003: 0003-disable-eigrp-crypto.patch
Patch0004: 0004-fips-mode.patch Patch0004: 0004-fips-mode.patch
Patch0005: 0005-icc-options.patch Patch0005: 0005-icc-options.patch
Patch0006: 0006-move-to-libexec.patch Patch0006: 0006-move-to-libexec.patch
Patch0007: 0007-ospfd-crash.patch
BuildRequires: autoconf BuildRequires: autoconf
BuildRequires: automake BuildRequires: automake
@ -203,6 +204,9 @@ fi
%{_sysusersdir}/%{name}.conf %{_sysusersdir}/%{name}.conf
%changelog %changelog
* Tue Jul 20 2021 Michal Ruprich <mruprich@redhat.com> - 7.5.1-7
- Resolves: #1983278 - ospfd crashes in route_node_delete with assertion fail
* Sat Jul 10 2021 Björn Esser <besser82@fedoraproject.org> - 7.5.1-6 * Sat Jul 10 2021 Björn Esser <besser82@fedoraproject.org> - 7.5.1-6
- Rebuild for versioned symbols in json-c - Rebuild for versioned symbols in json-c