| Message ID | 20240916202341.238735-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9a6a84a8a2b9336c2cdb943146207cb8a5a5260c |
| Headers | show |
| Series | [BlueZ,v2,1/2] shared/uhid: Fix crash after bt_uhid_unregister_all | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=890773 ---Test result--- Test Summary: CheckPatch PASS 0.59 seconds GitLint PASS 0.42 seconds BuildEll PASS 24.58 seconds BluezMake PASS 1637.31 seconds MakeCheck PASS 13.11 seconds MakeDistcheck PASS 178.59 seconds CheckValgrind PASS 252.10 seconds CheckSmatch PASS 353.91 seconds bluezmakeextell PASS 119.54 seconds IncrementalBuild PASS 2900.41 seconds ScanBuild PASS 1010.87 seconds --- Regards, Linux Bluetooth
Hello: This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 16 Sep 2024 16:23:40 -0400 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This fixes the following crash which happens when > bt_uhid_unregister_all is called from a notification callback: > > Invalid read of size 8 > at 0x1D9EFF: queue_foreach (queue.c:206) > by 0x1DEE58: uhid_read_handler (uhid.c:164) > Address 0x51286d8 is 8 bytes inside a block of size 16 free'd > at 0x48478EF: free (vg_replace_malloc.c:989) > by 0x1DA08D: queue_remove_if (queue.c:292) > by 0x1DA12F: queue_remove_all (queue.c:321) > by 0x1DE592: bt_uhid_unregister_all (uhid.c:300) > > [...] Here is the summary with links: - [BlueZ,v2,1/2] shared/uhid: Fix crash after bt_uhid_unregister_all https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9a6a84a8a2b9 - [BlueZ,v2,2/2] test-uhid: Add call to bt_uhid_unregister_all https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f9f98c0b2aa4 You are awesome, thank you!
diff --git a/src/shared/uhid.c b/src/shared/uhid.c index ed21e1399ba7..20bd26781799 100644 --- a/src/shared/uhid.c +++ b/src/shared/uhid.c @@ -42,6 +42,7 @@ struct bt_uhid { int ref_count; struct io *io; unsigned int notify_id; + bool notifying; struct queue *notify_list; struct queue *input; uint8_t type; @@ -56,6 +57,7 @@ struct uhid_notify { uint32_t event; bt_uhid_callback_t func; void *user_data; + bool removed; }; static void uhid_replay_free(struct uhid_replay *replay) @@ -134,6 +136,28 @@ static int bt_uhid_record(struct bt_uhid *uhid, bool input, return 0; } +static bool match_removed(const void *a, const void *b) +{ + const struct uhid_notify *notify = a; + + return notify->removed; +} + +static void uhid_notify(struct bt_uhid *uhid, struct uhid_event *ev) +{ + /* Add a reference to the uhid to ensure it doesn't get freed while at + * notify_handler. + */ + bt_uhid_ref(uhid); + + uhid->notifying = true; + queue_foreach(uhid->notify_list, notify_handler, ev); + uhid->notifying = false; + queue_remove_all(uhid->notify_list, match_removed, NULL, free); + + bt_uhid_unref(uhid); +} + static bool uhid_read_handler(struct io *io, void *user_data) { struct bt_uhid *uhid = user_data; @@ -161,7 +185,7 @@ static bool uhid_read_handler(struct io *io, void *user_data) break; } - queue_foreach(uhid->notify_list, notify_handler, &ev); + uhid_notify(uhid, &ev); return true; } @@ -292,13 +316,30 @@ static bool match_not_id(const void *a, const void *b) return notify->id != id; } +static void uhid_notify_removed(void *data, void *user_data) +{ + struct uhid_notify *notify = data; + struct bt_uhid *uhid = user_data; + + /* Skip marking start_id as removed since that is not removed with + * unregister all. + */ + if (notify->id == uhid->start_id) + return; + + notify->removed = true; +} + bool bt_uhid_unregister_all(struct bt_uhid *uhid) { if (!uhid) return false; - queue_remove_all(uhid->notify_list, match_not_id, + if (!uhid->notifying) + queue_remove_all(uhid->notify_list, match_not_id, UINT_TO_PTR(uhid->start_id), free); + else + queue_foreach(uhid->notify_list, uhid_notify_removed, uhid); return true; } @@ -588,7 +629,7 @@ resend: return 0; } - queue_foreach(uhid->notify_list, notify_handler, ev); + uhid_notify(uhid, ev); return 0; }
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This fixes the following crash which happens when bt_uhid_unregister_all is called from a notification callback: Invalid read of size 8 at 0x1D9EFF: queue_foreach (queue.c:206) by 0x1DEE58: uhid_read_handler (uhid.c:164) Address 0x51286d8 is 8 bytes inside a block of size 16 free'd at 0x48478EF: free (vg_replace_malloc.c:989) by 0x1DA08D: queue_remove_if (queue.c:292) by 0x1DA12F: queue_remove_all (queue.c:321) by 0x1DE592: bt_uhid_unregister_all (uhid.c:300) Fixes: https://github.com/bluez/bluez/issues/952 --- src/shared/uhid.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-)