linux-kernel - [PATCH 1/2] Bluetooth: Fix index added after unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220602094645.1.I7d191480c15b45a237b927e26aa26ba806409efb@changeid>
Date:   Thu,  2 Jun 2022 09:46:49 -0700
From:   Abhishek Pandit-Subedi <abhishekpandit@...gle.com>
To:     linux-bluetooth@...r.kernel.org, marcel@...tmann.org,
        luiz.dentz@...il.com
Cc:     Abhishek Pandit-Subedi <abhishekpandit@...omium.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: [PATCH 1/2] Bluetooth: Fix index added after unregister

From: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>

When a userchannel socket is released, we should check whether the hdev
is already unregistered before sending out an IndexAdded.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
---
This happened when the firmware crashed or the controller was lost for
some other reason.

For testing, I emualated this using:
echo 0 > $(readlink -f /sys/class/bluetooth/hci0)/../../authorized

   = Close Index: F8:E4:E3:D9:9E:45                     [hci0] 682.178794
    @ MGMT Event: Index Removed (0x0005) plen 0 {0x0002} [hci0] 682.178809
    @ MGMT Event: Index Removed (0x0005) plen 0 {0x0001} [hci0] 682.178809
    = Delete Index: F8:E4:E3:D9:9E:45                    [hci0] 682.178821
    @ USER Close: bt_stack_manage               {0x0003} [hci0] 682.397653
    @ MGMT Event: Index Added (0x0004) plen 0   {0x0002} [hci0] 682.397667
    @ MGMT Event: Index Added (0x0004) plen 0   {0x0001} [hci0] 682.397667
    @ MGMT Close: bt_stack_manage               {0x0002} 682.397793
    @ MGMT Open: bt_stack_manage (privileged) version 1.14     {0x0003} 682.437223
    @ MGMT Command: Read Controller Index List (0x0003) plen 0 {0x0003} 682.437230
    @ MGMT Event: Command Complete (0x0001) plen 5             {0x0003} 682.437232
          Read Controller Index List (0x0003) plen 2
            Status: Success (0x00)
            Controllers: 0

Tested on ChromeOS kernel and compiled with allmodconfig on
bluetooth-next.

 net/bluetooth/hci_sock.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 189e3115c8c6..bd8358b44aa4 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -869,7 +869,8 @@ static int hci_sock_release(struct socket *sock)
 
 	hdev = hci_pi(sk)->hdev;
 	if (hdev) {
-		if (hci_pi(sk)->channel == HCI_CHANNEL_USER) {
+		if (hci_pi(sk)->channel == HCI_CHANNEL_USER &&
+		    !hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
 			/* When releasing a user channel exclusive access,
 			 * call hci_dev_do_close directly instead of calling
 			 * hci_dev_close to ensure the exclusive access will
@@ -878,6 +879,11 @@ static int hci_sock_release(struct socket *sock)
 			 * The checking of HCI_AUTO_OFF is not needed in this
 			 * case since it will have been cleared already when
 			 * opening the user channel.
+			 *
+			 * Make sure to also check that we haven't already
+			 * unregistered since all the cleanup will have already
+			 * been complete and hdev will get released when we put
+			 * below.
 			 */
 			hci_dev_do_close(hdev);
 			hci_dev_clear_flag(hdev, HCI_USER_CHANNEL);
-- 
2.36.1.255.ge46751e96f-goog