proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-07-30 04:21:00 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #10541 from FRRouting/mergify/bp/dev/8.2/pr-10161

Browse Source 
zebra: Fix improper usage of hash_iterate that caused crashes (backport #10161)
This commit is contained in:
Donatas Abraitis 2022-02-09 15:15:31 +02:00 committed by GitHub
commit 7e38a5f2f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/hash.h
View File

@ -235,9 +235,10 @@ extern void *hash_release(struct hash *hash, void *data);
/* /*
* Iterate over the elements in a hash table. * Iterate over the elements in a hash table.
* *
* It is safe to delete items passed to the iteration function from the hash * The passed in arg to the handler function is the only safe
* table during iteration. More than one item cannot be deleted during each * item to delete from the hash.
* iteration. Please note that adding entries to the hash *
* Please note that adding entries to the hash
* during the walk will cause undefined behavior in that some new entries * during the walk will cause undefined behavior in that some new entries
* will be walked and some will not. So do not do this. * will be walked and some will not. So do not do this.
* *
@ -258,8 +259,10 @@ extern void hash_iterate(struct hash *hash,
/* /*
* Iterate over the elements in a hash table, stopping on condition. * Iterate over the elements in a hash table, stopping on condition.
* *
* It is safe to delete items passed to the iteration function from the hash * The passed in arg to the handler function is the only safe item
* table during iteration. Please note that adding entries to the hash * to delete from the hash.
*
* Please note that adding entries to the hash
* during the walk will cause undefined behavior in that some new entries * during the walk will cause undefined behavior in that some new entries
* will be walked and some will not. So do not do this. * will be walked and some will not. So do not do this.
* *

31
zebra/zebra_nhg.c
View File

@ -2995,7 +2995,7 @@ void zebra_nhg_dplane_result(struct zebra_dplane_ctx *ctx)
dplane_ctx_fini(&ctx); dplane_ctx_fini(&ctx);
} }
static void zebra_nhg_sweep_entry(struct hash_bucket *bucket, void *arg) static int zebra_nhg_sweep_entry(struct hash_bucket *bucket, void *arg)
{ {
struct nhg_hash_entry *nhe = NULL; struct nhg_hash_entry *nhe = NULL;
@ -3009,7 +3009,7 @@ static void zebra_nhg_sweep_entry(struct hash_bucket *bucket, void *arg)
* from an upper level proto. * from an upper level proto.
*/ */
if (zrouter.startup_time < nhe->uptime) if (zrouter.startup_time < nhe->uptime)
return; return HASHWALK_CONTINUE;
/* /*
* If it's proto-owned and not being used by a route, remove it since * If it's proto-owned and not being used by a route, remove it since
@ -3019,20 +3019,41 @@ static void zebra_nhg_sweep_entry(struct hash_bucket *bucket, void *arg)
*/ */
if (PROTO_OWNED(nhe) && nhe->refcnt == 1) { if (PROTO_OWNED(nhe) && nhe->refcnt == 1) {
zebra_nhg_decrement_ref(nhe); zebra_nhg_decrement_ref(nhe);
return; return HASHWALK_ABORT;
} }
/* /*
* If its being ref'd by routes, just let it be uninstalled via a route * If its being ref'd by routes, just let it be uninstalled via a route
* removal. * removal.
*/ */
if (ZEBRA_NHG_CREATED(nhe) && nhe->refcnt <= 0) if (ZEBRA_NHG_CREATED(nhe) && nhe->refcnt <= 0) {
zebra_nhg_uninstall_kernel(nhe); zebra_nhg_uninstall_kernel(nhe);
return HASHWALK_ABORT;
}
return HASHWALK_CONTINUE;
} }
void zebra_nhg_sweep_table(struct hash *hash) void zebra_nhg_sweep_table(struct hash *hash)
{ {
hash_iterate(hash, zebra_nhg_sweep_entry, NULL); uint32_t count;
/*
* Yes this is extremely odd. Effectively nhg's have
* other nexthop groups that depend on them and when you
* remove them, you can have other entries blown up.
* our hash code does not work with deleting multiple
* entries at a time and will possibly cause crashes
* So what to do? Whenever zebra_nhg_sweep_entry
* deletes an entry it will return HASHWALK_ABORT,
* cause that deletion might have triggered more.
* then we can just keep sweeping this table
* until nothing more is found to do.
*/
do {
count = hashcount(hash);
hash_walk(hash, zebra_nhg_sweep_entry, NULL);
} while (count != hashcount(hash));
} }
static void zebra_nhg_mark_keep_entry(struct hash_bucket *bucket, void *arg) static void zebra_nhg_mark_keep_entry(struct hash_bucket *bucket, void *arg)