public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [bug report] Bluetooth: hci_sync: Rework hci_suspend_notifier
@ 2022-07-27 11:58 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2022-07-27 11:58 UTC (permalink / raw)
  To: luiz.von.dentz; +Cc: linux-bluetooth

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-07-27 11:58 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-27 11:58 [bug report] Bluetooth: hci_sync: Rework hci_suspend_notifier Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox