Skip to content

lib: add missing hook_unregister_arg in mgmt_be_client_destroy#21940

Merged
Jafaral merged 1 commit into
FRRouting:masterfrom
routingrocks:fix/mgmt-be-hook-unregister
May 13, 2026
Merged

lib: add missing hook_unregister_arg in mgmt_be_client_destroy#21940
Jafaral merged 1 commit into
FRRouting:masterfrom
routingrocks:fix/mgmt-be-hook-unregister

Conversation

@routingrocks
Copy link
Copy Markdown
Contributor

Issue:
mgmt_be_client_create() registers a hook on nb_notification_tree_send but mgmt_be_client_destroy() never unregisters it. During shutdown, the client struct gets freed while the hook still points to it. When the subscription wheel timer fires after that, it dispatches to mgmt_be_send_notification() with the freed pointer and crashes.

  ZEBRA: Received signal 11 (si_addr 0x108000001d0)
  libfrr.so.0(nb_notification_tree_send+0x69)
  libfrr.so.0(nb_notification_send+0x165)
  libfrr.so.0(nb_notify_subscriptions+0xb8)
  libfrr.so.0(event_call+0x81)
  in thread wheel_timer_thread scheduled from ../lib/wheel.c:50

Fix by adding hook_unregister_arg() before freeing the client.

Signed-off-by: Rajesh Varatharaj rvaratharaj@nvidia.com

mgmt_be_client_create() registers a hook on nb_notification_tree_send
but mgmt_be_client_destroy() never unregisters it. During shutdown,
the client struct gets freed while the hook still points to it. When
the subscription wheel timer fires after that, it dispatches to
mgmt_be_send_notification() with the freed pointer and crashes.

  ZEBRA: Received signal 11 (si_addr 0x108000001d0)
  libfrr.so.0(nb_notification_tree_send+0x69)
  libfrr.so.0(nb_notification_send+0x165)
  libfrr.so.0(nb_notify_subscriptions+0xb8)
  libfrr.so.0(event_call+0x81)
  in thread wheel_timer_thread scheduled from ../lib/wheel.c:50

Fix by adding hook_unregister_arg() before freeing the client.

Signed-off-by: Rajesh Varatharaj <rvaratharaj@nvidia.com>
@frrbot frrbot Bot added the libfrr label May 13, 2026
@routingrocks routingrocks marked this pull request as draft May 13, 2026 16:11
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR fixes a use-after-free crash in mgmt_be_client_destroy() by adding the missing hook_unregister_arg() call before the client struct is freed. The hook registered in mgmt_be_client_create() on nb_notification_tree_send was never removed on teardown, leaving a dangling pointer that could be dereferenced by the subscription wheel timer after shutdown.

  • Adds hook_unregister_arg(nb_notification_tree_send, mgmt_be_send_notification, client) at the top of mgmt_be_client_destroy(), correctly placed before any cleanup or XFREE calls so the client pointer is still valid when the unregister happens.
  • The change is minimal, targeted, and mirrors the existing hook_register_arg call in the create path.

Confidence Score: 5/5

Safe to merge — the change adds the single missing teardown call that was already expected by the symmetric create path.

The fix is a one-to-one complement of the existing hook_register_arg call in mgmt_be_client_create(). The unregister is placed before any XFREE call, so the client pointer is valid at the time of unregistration. There are no other hook registrations in this file left without matching unregistrations.

No files require special attention.

Important Files Changed

Filename Overview
lib/mgmt_be_client.c Adds hook_unregister_arg() before client cleanup in mgmt_be_client_destroy(), fixing a use-after-free crash; placement is correct and the fix is complete.

Reviews (2): Last reviewed commit: "lib: add missing hook_unregister_arg in ..." | Re-trigger Greptile

@routingrocks routingrocks marked this pull request as ready for review May 13, 2026 18:16
@Jafaral
Copy link
Copy Markdown
Member

Jafaral commented May 13, 2026

@Mergifyio backport stable/10.6 stable/10.5 stable/10.4

@Jafaral Jafaral merged commit 992a350 into FRRouting:master May 13, 2026
25 checks passed
@mergify
Copy link
Copy Markdown

mergify Bot commented May 13, 2026

backport stable/10.6 stable/10.5 stable/10.4

✅ Backports have been created

Details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants