[PATCH v3] Bluetooth: btnxpuart: Validate the FW dump header length
Ali Ahmet Memis <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.bluez.kernel |
|---|---|
| Message-ID | <[email protected]> |
nxp_process_fw_dump() pulls the ACL header off the frame and then reads
seq_num and buf_len from a struct nxp_fw_dump_hdr placed at skb->data,
without checking that the ACL payload is long enough to contain it.
h4_recv_buf() collects HCI_ACL_HDR_SIZE bytes of header followed by the
number of payload bytes named in that header, so skb->len is 4 + dlen
with dlen supplied by the controller and possibly smaller than the 8
byte dump header, or zero. A short frame with connection handle 0xfff
therefore reads both fields from beyond the received data.
Beyond the read itself, buf_len is what terminates a dump: a value of
zero makes the driver call hci_devcd_complete() and reset the
controller, so a truncated frame can end a dump early.
Use skb_pull_data() to validate and pull the FW dump header before
accessing its fields. Warn and reject the chunk if the header is
truncated.
Fixes: 998e447f443f ("Bluetooth: btnxpuart: Add support for HCI coredump feature")
Signed-off-by: Ali Ahmet Memis <[email protected]>
---
v3: Use skb_pull_data() to validate and pull the FW dump header, as
suggested by Luiz.
v2: Warn on the early exit path instead of dropping the frame silently,
as suggested by Neeraj. Dropped the trailing newline from the
suggested message, since bt_dev_warn() already appends one.
v1: https://lore.kernel.org/all/[email protected]/
drivers/bluetooth/btnxpuart.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 6a1cffe08d5f..e6c15bc6a30b 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1368,12 +1368,21 @@ static int nxp_process_fw_dump(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_acl_hdr *acl_hdr = (struct hci_acl_hdr *)skb_pull_data(skb,
sizeof(*acl_hdr));
- struct nxp_fw_dump_hdr *fw_dump_hdr = (struct nxp_fw_dump_hdr *)skb->data;
+ struct nxp_fw_dump_hdr *fw_dump_hdr;
struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
- __u16 seq_num = __le16_to_cpu(fw_dump_hdr->seq_num);
- __u16 buf_len = __le16_to_cpu(fw_dump_hdr->buf_len);
+ __u16 seq_num;
+ __u16 buf_len;
int err;
+ fw_dump_hdr = skb_pull_data(skb, sizeof(*fw_dump_hdr));
+ if (!fw_dump_hdr) {
+ bt_dev_warn(hdev, "FW dump: invalid or corrupt fw dump chunk");
+ goto free_skb;
+ }
+
+ seq_num = __le16_to_cpu(fw_dump_hdr->seq_num);
+ buf_len = __le16_to_cpu(fw_dump_hdr->buf_len);
+
if (seq_num == 0x0001) {
if (test_and_set_bit(BTNXPUART_FW_DUMP_IN_PROGRESS, &nxpdev->tx_state)) {
bt_dev_err(hdev, "FW dump already in progress");
--
2.55.0