[bluez/bluez] 639385: shared/mgmt: Fix notify leak in mgmt_unregister()

[bluez/bluez] 639385: shared/mgmt: Fix notify leak in mgmt_unregister()

[IAROSLAV VOITOVYCH]

  Branch: refs/heads/1181228
  Home:   https://github.com/bluez/bluez
  Commit: 6393853cec256db4d656c7f17351ccad324f7f1c
      https://github.com/bluez/bluez/commit/6393853cec256db4d656c7f17351ccad324f7f1c
  Author: Iaroslav Voitovych <yaroslav.voytovych@gmail.com>
  Date:   2026-10-08 (Thu, 08 Oct 2026)

  Changed paths:
    M src/shared/mgmt.c

  Log Message:
  -----------
  shared/mgmt: Fix notify leak in mgmt_unregister()

mgmt_unregister() called from a notification callback takes the entry
off notify_list and marks it removed instead of freeing it.
process_notify() frees the removed entries still on notify_list once
the walk is done, so this one is never freed and its destroy callback
never runs.

If the callback unregisters the next entry rather than its own, this
is also a use-after-free: queue_foreach() has already saved that
entry's queue node, and queue_remove_if() frees it. That is the crash
fixed for mgmt_unregister_index() by commit 872729a91632
("shared/mgmt: Fix crash when removing index"), still present in
mgmt_unregister().

Leave the entry on the list while notifying, as mgmt_unregister_index()
and mgmt_unregister_all() do, and let process_notify() free it.
Unregistering an entry that is already marked removed now returns
false, since it is no longer registered; for an entry marked by
mgmt_unregister_index() or mgmt_unregister_all(), mgmt_unregister()
used to return true, take it off the list and leak it. No caller
checks the return value.

mgmt-tester, mesh-tester and userchan-tester unregister from event
callbacks, and under ASan each reports these entries as leaked, for
example userchan-tester:

  Direct leak of 80 byte(s) in 2 object(s) allocated from:
    #0 in malloc
    #1 in util_malloc src/shared/util.c:46
    #2 in mgmt_register src/shared/mgmt.c:974
    #3 in test_open_success tools/userchan-tester.c:275

Fixes: 872729a91632 ("shared/mgmt: Fix crash when removing index")


  Commit: 5b628cd3c7935db169bc49bd3887624f5d1bac54
      https://github.com/bluez/bluez/commit/5b628cd3c7935db169bc49bd3887624f5d1bac54
  Author: Iaroslav Voitovych <yaroslav.voytovych@gmail.com>
  Date:   2026-10-08 (Thu, 08 Oct 2026)

  Changed paths:
    M unit/test-mgmt.c

  Log Message:
  -----------
  unit/test-mgmt: Test unregistering from callbacks

Add /mgmt/unregister/3 to /5, where a notification callback
unregisters its own entry, the next entry, or another entry followed
by mgmt_unregister_all(). The unregistered entry must not be notified
again, its destroy callback must not run inside the notification and
must have run once by the next idle callback, and unregistering it
again must fail.

Add /mgmt/unregister/6, which unregisters outside a notification: the
entry is destroyed at once and unregistering it again fails.

Without the previous patch, /4 faults under ASan (heap-use-after-free
in queue_foreach()), /3 never destroys the entry, and /5 gets true for
an entry already marked removed.


Compare: https://github.com/bluez/bluez/compare/6393853cec25%5E...5b628cd3c793

To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications