All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Marcel Holtmann <marcel@holtmann.org>,
	Loic Poulain <loic.poulain@intel.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
	Loic Poulain <loic.poulain@intel.com>,
	linux-bluetooth@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH] Bluetooth: hci_intel: prevent reads beyond the end of skb->data
Date: Thu, 27 May 2021 16:03:31 +0300	[thread overview]
Message-ID: <YK+Yo6c1UuiACSZA@mwanda> (raw)

There doesn't appear to be any checks to ensure that skb->data is large
enough in these functions.  For most of these, if we specify a header
length, then h4_recv_buf() will ensure that all packets are at least the
minimum length.  The intel_recv_lpm() function needs an additional
check for LPM_OP_TX_NOTIFY packets.

Fixes: ca93cee5a56e ("Bluetooth: hci_uart: Add basic support for Intel Lightning Peak devices")

No signed-off-by because I can't test this and just wanted to collect
feedback.  This is part of a static checker warning because someone
reported the hci_event.c read overflows to security@kernel.org.  This
stuff is quite complicated for static checkers of course and I don't
understand all the rules yet.  Right now I have about 2000 warnings
that look like this:

drivers/bluetooth/hci_intel.c:877 intel_recv_event() warn: assignment assumes 'skb->len' is '2' bytes
drivers/bluetooth/hci_intel.c:922 intel_recv_lpm() warn: assignment assumes 'skb->len' is '2' bytes
drivers/bluetooth/hci_intel.c:1028 intel_dequeue() warn: assignment assumes 'skb->len' is '3' bytes

I think there should be a different additional static checker warning
for h4_recv_pkt structs like in this patch if you fail to specify a
.hlen value?

regards,
dan carpenter
---
 drivers/bluetooth/hci_intel.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 7249b91d9b91..3e4bccacad9b 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -925,7 +925,7 @@ static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
 
 	switch (lpm->opcode) {
 	case LPM_OP_TX_NOTIFY:
-		if (lpm->dlen < 1) {
+		if (lpm->dlen < 1 || skb->len < struct_size(lpm, data, 1)) {
 			bt_dev_err(hu->hdev, "Invalid LPM notification packet");
 			break;
 		}
@@ -959,10 +959,10 @@ static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
 	.maxlen = HCI_LPM_MAX_SIZE
 
 static const struct h4_recv_pkt intel_recv_pkts[] = {
-	{ H4_RECV_ACL,    .recv = hci_recv_frame   },
-	{ H4_RECV_SCO,    .recv = hci_recv_frame   },
-	{ H4_RECV_EVENT,  .recv = intel_recv_event },
-	{ INTEL_RECV_LPM, .recv = intel_recv_lpm   },
+	{ H4_RECV_ACL,    .recv = hci_recv_frame, .hlen = sizeof(struct bt_skb_cb) },
+	{ H4_RECV_SCO,    .recv = hci_recv_frame, .hlen = sizeof(struct bt_skb_cb) },
+	{ H4_RECV_EVENT,  .recv = intel_recv_event, .hlen = sizeof(struct hci_event_hdr) },
+	{ INTEL_RECV_LPM, .recv = intel_recv_lpm, .hlen = sizeof(struct hci_lpm_pkt) },
 };
 
 static int intel_recv(struct hci_uart *hu, const void *data, int count)
-- 
2.30.2


             reply	other threads:[~2021-05-27 13:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-27 13:03 Dan Carpenter [this message]
2021-05-27 13:55 ` Bluetooth: hci_intel: prevent reads beyond the end of skb->data bluez.test.bot
2021-05-27 15:19 ` [PATCH] " Marcel Holtmann
2021-05-27 16:49   ` Dan Carpenter

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=YK+Yo6c1UuiACSZA@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=johan.hedberg@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=loic.poulain@intel.com \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.