Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <436acf8a-ea5f-a308-0e3e-fc7c6ffde7aa@ugent.be>
Date:   Wed, 9 Feb 2022 15:08:51 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.1
Content-Language: en-US
To:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>
From:   Niels Dossche <niels.dossche@ugent.be>
Subject: [PATCH] Bluetooth: hci_event: Add missing locking on hdev in
 hci_le_ext_adv_term_evt
Cc:     linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b0Q3V3hnckY0aXFWRU9BU3Z0ZmJwYzY0TkxKZmVaaWhHYkJESkZkSnJTQjVo?=
 =?utf-8?B?TDNQUFBieTZ1b0hNZTVaanlDdXBPdldWQzgxTk43ZUJvVFp0VzJzK2FlSUgr?=
 =?utf-8?B?REV0Vm1yZ0dVT1hRc2pTVkViZ1lTUTNydDk5OFZIUjBuTGc1bDgxVmx4Z2l1?=
 =?utf-8?B?QjB6cDY0OVpKTUNtbHBtZGtHejMxVy9WaVVYcG1oaWRja3BMbUx6VUhOaElG?=
 =?utf-8?B?UzA4SFprT1dLUXRFeXh1Y3h1MmlsenpBSEVGYXFQVmhXNVJLQkhIZkx4UzVR?=
 =?utf-8?B?NzZ5NE9Eb0RqQ0thTHJpNHIwQ1dSMzh0UFdHOWR1RGkzcUtseXQzTmVaZ2F4?=
 =?utf-8?B?SjZjcmhuMVVzM1JNVTJlWG5SSnl3US9mVkZGclpaLzV5bE91alR1NFlpdVUz?=
 =?utf-8?B?dWRwM0IwWGZHa1I1MzhzdWphRDNYWHRXc1JBaDBpUG5BTVQwaG9zenNNbTkz?=
 =?utf-8?B?ZFQrMnAzNkhMalcwQXNCOUNkekwrLy8vNXFnWjV1Um9DN3djM3pqa1JOc011?=
 =?utf-8?B?dXVnQ0FUeVpYY0dwMDBjTE9BcGk0T2t5Z0FocTBOb2tEczhoS202QlRBaDNW?=
 =?utf-8?B?d3U5N1BXQko4OEZzQnhTdWRkSmNid0tvN3JzSEhZUHFmOTZZcVA0ZDdlS3c1?=
 =?utf-8?B?dHlXYzVBd0JMelFWTDJ5V1F0aW80ZmV2NkNoQXV0UzVRSUtnZFB3RTNTVGRU?=
 =?utf-8?B?dGJNQkU4Znk4RmJyeHFQRjRNdzV1SWxmbkRkUGxiSW5EMVArQVh0NS93WWhX?=
 =?utf-8?B?Wjh6eHRUYUVTWE1zd0lkdnZ3R0tIMjJtdDJKbC9EL3QwSnRONWhWUUdUR0xZ?=
 =?utf-8?B?UVBMemhETWd1cVlGbTFjV1h5VlNYdTlWZ2k3TS8wYUNVaHpVdWszc0RyMlAx?=
 =?utf-8?B?ZlBwYmVBQXIydjFSdk9IaHBDMmZ4dXR5VU9WMzg0Mm9Fa1gwNXRhdzlpOXpz?=
 =?utf-8?B?bDVHVDV3ZCs5WGZXYk8zM1RQOS9MT1AvNDQ3cUUrTVVaVEViNGxxU0lRSkpF?=
 =?utf-8?B?cHQwSnNXQTE5SmhCY0s2NWxvR0xSVWJhZ0l2elMzZDVhVHNsK3NTKzIvbm1M?=
 =?utf-8?B?SUdVdUZtRDVySVk3NFkzZDdCaUNHZ2tuYy9hTUVDT3g5TS9rRjZtVUh6dWJO?=
 =?utf-8?B?Zm0yL3ZIakJYMTh2Q2dIVmM4RUNabnNRaXdTRFBGUEorMWFFUjMzMlVvN2tR?=
 =?utf-8?B?ajZkSjJBdExOK3EwUnVMVDZhL3QzVkw3TmQ2cWo5bCt1Z0wzTkhpSWxoN1NY?=
 =?utf-8?B?a2FVTXJBaHo0bC9MSzZKemlmTkhYd3BPdWZ6dWwvSWp2cndINUszK3ltZXlm?=
 =?utf-8?B?T0ozZ1pGcjFzdHFoVitONkdTM2EreWxjdThLbTJhcHpxQ0ZCSVB0WHVLT2Jo?=
 =?utf-8?B?YXY5SjdsWDNiWSs0emJZUHhHWjcyT1BHRWdCTE1GSE5DSkFiYVgvdExaNXJw?=
 =?utf-8?B?SDlSK0p6WEpuMjZCa0graHVHU3U3TVVXT3NiMjNLTXlCd3J6T1BpaXVoZHNN?=
 =?utf-8?B?M2YvY09aRUh6QnllWHhJQ200MCs1MnBMQWNtZFlHdVFIY0VwRHRvWW5CSURC?=
 =?utf-8?B?VVFnYkp3M2M1YzVIN21SZ1hWS3ZLMEJOZUh0ZVBZSWtJckp0RGFyTVJacXZ4?=
 =?utf-8?B?TEVwS2laVC90QXFCUjY0R1UySVU1SURCay9WWGJhMjhUeGdXOEFXYncyU2ZM?=
 =?utf-8?B?SmNJNTVNaHc0MldxZEE3bnVHcEdQTVRNM1plWlVMMTdsaktTWDVEekxLczdQ?=
 =?utf-8?B?emV0dkQwaU93K2ZCTEQzbHoycEJReHE4bnN5cnNiRTdsNDBSSm54RU53K0hF?=
 =?utf-8?B?ZklMeXhkTUViK2MxR1FQMnJmcmdKM2NFZ3FtUmV6aWVXRGp4V1U0NTh0WXE0?=
 =?utf-8?B?UjdWY0lEZVRjbm05cjhJU2pjZGxucmdvd1duczFIT2ZXMmNUTnVCYzc3cDhJ?=
 =?utf-8?B?a3pWNGh3bHp4LzdGL3ROUDVZcm9vVUVUZ2I3SnJLYXNXTUdkNkVmaDNWQWIv?=
 =?utf-8?B?ZUVHclRqNDg2dG9td09JMDB2NXJ0OCt0Q3QySUlLQkMrMHpKNGtybnNCNzNm?=
 =?utf-8?B?OHhBcWhGY3VubzlGeDlIQUpUZStFWFJqZWcxWGEyZ015bDI5Y3lIZnlabEl1?=
 =?utf-8?B?TGVyRTdNNFNSVjBSejZaZHcramE0NGRYS2Q4ODRveVo5dXh1NnFhU1VIUU90?=
 =?utf-8?Q?ZY73ptExTpKBq5rFc7geO2I=3D?=
X-OriginatorOrg: ugent.be
X-MS-Exchange-CrossTenant-Network-Message-Id: 1de74638-90f2-4a62-90f7-08d9ebd5b436
X-MS-Exchange-CrossTenant-AuthSource: AM0PR09MB2324.eurprd09.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2022 14:08:53.5078
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: d7811cde-ecef-496c-8f91-a1786241b99c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Drslak3EZr0lfAX9hCq+pUKLsZ6D18+C9EwZYxAxaKlaMDHCgyzuMpbwy8bc5+4E4agVVX2vj0gilJl0QOGJ/g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0902MB2176
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

Both hci_find_adv_instance and hci_remove_adv_instance have a comment
above their function definition saying that these two functions require
the caller to hold the hdev->lock lock. However, hci_le_ext_adv_term_evt
does not acquire that lock and neither does its caller hci_le_meta_evt
(hci_le_meta_evt calls hci_le_ext_adv_term_evt via an indirect function
call because of the lookup in hci_le_ev_table).

The other event handlers all acquire and release the hdev->lock and they
follow the rule that hci_find_adv_instance and hci_remove_adv_instance
must be called while holding the hdev->lock lock.

The solution is to make sure hci_le_ext_adv_term_evt also acquires and
releases the hdev->lock lock. The check on ev->status which logs a
warning and does an early return is not covered by the lock because
other functions also access ev->status without holding the lock.

Signed-off-by: Niels Dossche <niels.dossche@ugent.be>
---
 net/bluetooth/hci_event.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index fc30f4c03d29..3bf048d0df37 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5670,8 +5670,6 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 
 	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
 
-	adv = hci_find_adv_instance(hdev, ev->handle);
-
 	/* The Bluetooth Core 5.3 specification clearly states that this event
 	 * shall not be sent when the Host disables the advertising set. So in
 	 * case of HCI_ERROR_CANCELLED_BY_HOST, just ignore the event.
@@ -5684,9 +5682,13 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 		return;
 	}
 
+	hci_dev_lock(hdev);
+
+	adv = hci_find_adv_instance(hdev, ev->handle);
+
 	if (ev->status) {
 		if (!adv)
-			return;
+			goto unlock;
 
 		/* Remove advertising as it has been terminated */
 		hci_remove_adv_instance(hdev, ev->handle);
@@ -5694,12 +5696,12 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 
 		list_for_each_entry_safe(adv, n, &hdev->adv_instances, list) {
 			if (adv->enabled)
-				return;
+				goto unlock;
 		}
 
 		/* We are no longer advertising, clear HCI_LE_ADV */
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);
-		return;
+		goto unlock;
 	}
 
 	if (adv)
@@ -5714,16 +5716,19 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 
 		if (hdev->adv_addr_type != ADDR_LE_DEV_RANDOM ||
 		    bacmp(&conn->resp_addr, BDADDR_ANY))
-			return;
+			goto unlock;
 
 		if (!ev->handle) {
 			bacpy(&conn->resp_addr, &hdev->random_addr);
-			return;
+			goto unlock;
 		}
 
 		if (adv)
 			bacpy(&conn->resp_addr, &adv->random_addr);
 	}
+
+unlock:
+	hci_dev_unlock(hdev);
 }
 
 static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, void *data,
-- 
2.35.1