2022-07-27 12:01:43

by Dan Carpenter

[permalink] [raw]
Subject: [bug report] Bluetooth: hci_sync: Rework hci_suspend_notifier

Hello Luiz Augusto von Dentz,

The patch 182ee45da083: "Bluetooth: hci_sync: Rework
hci_suspend_notifier" from Oct 27, 2021, leads to the following
Smatch static checker warning:

net/bluetooth/hci_core.c:1944 hci_add_adv_monitor()
error: dereferencing freed memory 'monitor'

net/bluetooth/hci_core.c
1942 case HCI_ADV_MONITOR_EXT_MSFT:
1943 status = msft_add_monitor_pattern(hdev, monitor);
--> 1944 bt_dev_dbg(hdev, "%s add monitor %d msft status %d", hdev->name,
1945 monitor->handle, status);
1946 break;
1947 }

The problem is that msft_le_monitor_advertisement_cb() will free
monitor on error.

net/bluetooth/msft.c
243 monitor->state = ADV_MONITOR_STATE_OFFLOADED;
244
245 unlock:
246 if (status)
247 hci_free_adv_monitor(hdev, monitor);
^^^^ ^^^^^^^

248
249 hci_dev_unlock(hdev);
250
251 return status;
252 }

I really think freeing monitor is the wrong thing. It's a layering
violation. Other error paths in the callers do not free monitor. It
leads to other use after frees besides this one that that static checker
found. For example, it leads to a double free in
__add_adv_patterns_monitor().

regards,
dan carpenter