linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Archie Pusaka <apusaka@google.com>
To: linux-bluetooth <linux-bluetooth@vger.kernel.org>,
	Marcel Holtmann <marcel@holtmann.org>
Cc: chromeos-bluetooth-upstreaming 
	<chromeos-bluetooth-upstreaming@chromium.org>,
	Archie Pusaka <apusaka@chromium.org>,
	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: [RFC PATCH v1 1/2] Bluetooth: queue ACL packets if no handle is found
Date: Sat, 27 Jun 2020 18:54:36 +0800	[thread overview]
Message-ID: <20200627185320.RFC.v1.1.Icea550bb064a24b89f2217cf19e35b4480a31afd@changeid> (raw)
In-Reply-To: <20200627105437.453053-1-apusaka@google.com>

From: Archie Pusaka <apusaka@chromium.org>

There is a possibility that an ACL packet is received before we
receive the HCI connect event for the corresponding handle. If this
happens, we discard the ACL packet.

Rather than just ignoring them, this patch provides a queue for
incoming ACL packet without a handle. The queue is processed when
receiving a HCI connection event. If 2 seconds elapsed without
receiving the HCI connection event, assume something bad happened
and discard the queued packet.

Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

---

 include/net/bluetooth/hci_core.h |  8 +++
 net/bluetooth/hci_core.c         | 84 +++++++++++++++++++++++++++++---
 net/bluetooth/hci_event.c        |  2 +
 3 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 836dc997ff94..b69ecdd0d15a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -270,6 +270,9 @@ struct adv_monitor {
 /* Default authenticated payload timeout 30s */
 #define DEFAULT_AUTH_PAYLOAD_TIMEOUT   0x0bb8
 
+/* Time to keep ACL packets without a corresponding handle queued (2s) */
+#define PENDING_ACL_TIMEOUT		msecs_to_jiffies(2000)
+
 struct amp_assoc {
 	__u16	len;
 	__u16	offset;
@@ -538,6 +541,9 @@ struct hci_dev {
 	struct delayed_work	rpa_expired;
 	bdaddr_t		rpa;
 
+	struct delayed_work	remove_pending_acl;
+	struct sk_buff_head	pending_acl_q;
+
 #if IS_ENABLED(CONFIG_BT_LEDS)
 	struct led_trigger	*power_led;
 #endif
@@ -1773,6 +1779,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
 void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			       u8 *bdaddr_type);
 
+void hci_process_pending_acl(struct hci_dev *hdev, struct hci_conn *conn);
+
 #define SCO_AIRMODE_MASK       0x0003
 #define SCO_AIRMODE_CVSD       0x0000
 #define SCO_AIRMODE_TRANSP     0x0003
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7959b851cc63..30780242c267 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1786,6 +1786,7 @@ int hci_dev_do_close(struct hci_dev *hdev)
 	skb_queue_purge(&hdev->rx_q);
 	skb_queue_purge(&hdev->cmd_q);
 	skb_queue_purge(&hdev->raw_q);
+	skb_queue_purge(&hdev->pending_acl_q);
 
 	/* Drop last sent command */
 	if (hdev->sent_cmd) {
@@ -3518,6 +3519,78 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action,
 	return NOTIFY_STOP;
 }
 
+static void hci_add_pending_acl(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	skb_queue_tail(&hdev->pending_acl_q, skb);
+
+	queue_delayed_work(hdev->workqueue, &hdev->remove_pending_acl,
+			   PENDING_ACL_TIMEOUT);
+}
+
+void hci_process_pending_acl(struct hci_dev *hdev, struct hci_conn *conn)
+{
+	struct sk_buff *skb, *tmp;
+	struct hci_acl_hdr *hdr;
+	u16 handle, flags;
+	bool reset_timer = false;
+
+	skb_queue_walk_safe(&hdev->pending_acl_q, skb, tmp) {
+		hdr = (struct hci_acl_hdr *)skb->data;
+		handle = __le16_to_cpu(hdr->handle);
+		flags  = hci_flags(handle);
+		handle = hci_handle(handle);
+
+		if (handle != conn->handle)
+			continue;
+
+		__skb_unlink(skb, &hdev->pending_acl_q);
+		skb_pull(skb, HCI_ACL_HDR_SIZE);
+
+		l2cap_recv_acldata(conn, skb, flags);
+		reset_timer = true;
+	}
+
+	if (reset_timer)
+		mod_delayed_work(hdev->workqueue, &hdev->remove_pending_acl,
+				 PENDING_ACL_TIMEOUT);
+}
+
+/* Remove the oldest pending ACL, and all pending ACLs with the same handle */
+static void hci_remove_pending_acl(struct work_struct *work)
+{
+	struct hci_dev *hdev;
+	struct sk_buff *skb, *tmp;
+	struct hci_acl_hdr *hdr;
+	u16 handle, oldest_handle;
+
+	hdev = container_of(work, struct hci_dev, remove_pending_acl.work);
+	skb = skb_dequeue(&hdev->pending_acl_q);
+
+	if (!skb)
+		return;
+
+	hdr = (struct hci_acl_hdr *)skb->data;
+	oldest_handle = hci_handle(__le16_to_cpu(hdr->handle));
+	kfree_skb(skb);
+
+	bt_dev_err(hdev, "ACL packet for unknown connection handle %d",
+		   oldest_handle);
+
+	skb_queue_walk_safe(&hdev->pending_acl_q, skb, tmp) {
+		hdr = (struct hci_acl_hdr *)skb->data;
+		handle = hci_handle(__le16_to_cpu(hdr->handle));
+
+		if (handle == oldest_handle) {
+			__skb_unlink(skb, &hdev->pending_acl_q);
+			kfree_skb(skb);
+		}
+	}
+
+	if (!skb_queue_empty(&hdev->pending_acl_q))
+		queue_delayed_work(hdev->workqueue, &hdev->remove_pending_acl,
+				   PENDING_ACL_TIMEOUT);
+}
+
 /* Alloc HCI device */
 struct hci_dev *hci_alloc_dev(void)
 {
@@ -3610,10 +3683,12 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_WORK(&hdev->suspend_prepare, hci_prepare_suspend);
 
 	INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
+	INIT_DELAYED_WORK(&hdev->remove_pending_acl, hci_remove_pending_acl);
 
 	skb_queue_head_init(&hdev->rx_q);
 	skb_queue_head_init(&hdev->cmd_q);
 	skb_queue_head_init(&hdev->raw_q);
+	skb_queue_head_init(&hdev->pending_acl_q);
 
 	init_waitqueue_head(&hdev->req_wait_q);
 	init_waitqueue_head(&hdev->suspend_wait_q);
@@ -4662,8 +4737,6 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	struct hci_conn *conn;
 	__u16 handle, flags;
 
-	skb_pull(skb, HCI_ACL_HDR_SIZE);
-
 	handle = __le16_to_cpu(hdr->handle);
 	flags  = hci_flags(handle);
 	handle = hci_handle(handle);
@@ -4678,17 +4751,16 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_dev_unlock(hdev);
 
 	if (conn) {
+		skb_pull(skb, HCI_ACL_HDR_SIZE);
+
 		hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF);
 
 		/* Send to upper protocol */
 		l2cap_recv_acldata(conn, skb, flags);
 		return;
 	} else {
-		bt_dev_err(hdev, "ACL packet for unknown connection handle %d",
-			   handle);
+		hci_add_pending_acl(hdev, skb);
 	}
-
-	kfree_skb(skb);
 }
 
 /* SCO data packet */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index e060fc9ebb18..108c6c102a6a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2627,6 +2627,8 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 			hci_send_cmd(hdev, HCI_OP_CHANGE_CONN_PTYPE, sizeof(cp),
 				     &cp);
 		}
+
+		hci_process_pending_acl(hdev, conn);
 	} else {
 		conn->state = BT_CLOSED;
 		if (conn->type == ACL_LINK)
-- 
2.27.0.212.ge8ba1cc988-goog


  reply	other threads:[~2020-06-27 10:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-27 10:54 [RFC PATCH v1 0/2] handle USB endpoint race condition Archie Pusaka
2020-06-27 10:54 ` Archie Pusaka [this message]
2020-06-29  6:40   ` [RFC PATCH v1 1/2] Bluetooth: queue ACL packets if no handle is found Marcel Holtmann
2020-06-30  6:48     ` Archie Pusaka
2020-06-30  6:54       ` Marcel Holtmann
     [not found]         ` <CALWDO_Vrn_pXMbkXifKFazha7BYPqLpCthqHOb9ZmVE3wDRMfA@mail.gmail.com>
2020-07-15 14:07           ` Alain Michaud
2020-07-15 15:25             ` Luiz Augusto von Dentz
2020-07-17  6:51           ` Marcel Holtmann
2020-07-17 13:17             ` Alain Michaud
2020-06-27 10:54 ` [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed Archie Pusaka
2020-06-29  6:49   ` Marcel Holtmann
2020-06-29 19:42     ` Luiz Augusto von Dentz
2020-06-29 20:20       ` Marcel Holtmann
2020-06-29 22:44         ` Luiz Augusto von Dentz
2020-06-30  6:48           ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200627185320.RFC.v1.1.Icea550bb064a24b89f2217cf19e35b4480a31afd@changeid \
    --to=apusaka@google.com \
    --cc=abhishekpandit@chromium.org \
    --cc=apusaka@chromium.org \
    --cc=chromeos-bluetooth-upstreaming@chromium.org \
    --cc=davem@davemloft.net \
    --cc=johan.hedberg@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).