From patchwork Thu Mar 21 06:12:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 1059822 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="vdYFeF3V"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44PxK80RVNz9sRc for ; Thu, 21 Mar 2019 17:12:28 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725995AbfCUGM0 (ORCPT ); Thu, 21 Mar 2019 02:12:26 -0400 Received: from mail-pg1-f169.google.com ([209.85.215.169]:46883 "EHLO mail-pg1-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726523AbfCUGMZ (ORCPT ); Thu, 21 Mar 2019 02:12:25 -0400 Received: by mail-pg1-f169.google.com with SMTP id a22so3484387pgg.13 for ; Wed, 20 Mar 2019 23:12:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=LXpjnoyyHaApsTZQjiHMJGTgFKQpepajk+n2atHMOLE=; b=vdYFeF3V37Xe/FxWAPzpUKuL37IW4JQZsA6ldPYCKHQBp4G2bQ10Po+mtXXa3KUpc0 8tL6JLnxgKLO/QPVaGUFkeGZ1YdEjFNOMzGF1TPANn+c0+/JcDytFeAE5uEWoq0UXkbZ NRz/6JldTLk2G8APe//nIIcj+9d+PIzx3/vMq0tmXb8kJgZggd3U/2YDNi4ghBCxIW0p GI0qdPMpFuxaZL6J/ZdpfOkWr3LJqFPks1fkUdSEJR6dyhuJHAJ64kyQQegkF3fsps4U og8Ijgql4KVJfs9a4JefYIMmahHQ6r+Z9tBjAiI2tFvGYEm/bPsChgoSggDdnzAezpuj S78Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=LXpjnoyyHaApsTZQjiHMJGTgFKQpepajk+n2atHMOLE=; b=obb1ozaJr3BrHryw8LqYYTyAARbgUtNSWD32Q454bNzQ3yLWXD8preux6kZXcrhJ66 f5v3kTqWd4N9pmUmWBbsPL1JySEd3k4PuSGWddvgd04g0BgCi55Vw6tzzoyNbgwM8FvR ZqB1aQweTc3qoTNbVVC2PsuHvHvKwSuledhGrHFG2iyrcRYKrNnQ+wdr4hEtPscAK+yP oysofonCsMyMTJPvPx6YSjUmj0IRFscxo35638HEvf8IUfR2vhNw5AfXcj5X+Ft3s2In ngYNgmWMgNlSqt52hKuLWj+V5rprgDil54Gp9PcCzf1p+0kzKyT2WfaNnsYRVbOL2r/L E59g== X-Gm-Message-State: APjAAAUbT6SJg9MajKB75/ycnYN9Dof84soLEtfgW2X3eDhEEL2ZtcDA rP3W9wSr6+QkGnZ+d2//BT0ohD5/ X-Google-Smtp-Source: APXvYqyf0t3nasWRlvjHDWO15reRy29s3C1XDSTReJY+pmo1zow5ck5NcWeeTJi1R3TK70tFJQddMg== X-Received: by 2002:a17:902:d70f:: with SMTP id w15mr1862528ply.234.1553148744935; Wed, 20 Mar 2019 23:12:24 -0700 (PDT) Received: from tw-172-25-17-123.office.twttr.net ([8.25.197.27]) by smtp.gmail.com with ESMTPSA id v4sm6594426pff.181.2019.03.20.23.12.23 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Mar 2019 23:12:24 -0700 (PDT) From: Cong Wang To: netdev@vger.kernel.org Cc: Cong Wang , Marcel Holtmann , Johan Hedberg Subject: [Patch net v2 2/3] bluetooth: validate HCI_EV_LE_META packet carefully Date: Wed, 20 Mar 2019 23:12:07 -0700 Message-Id: <20190321061208.16163-3-xiyou.wangcong@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190321061208.16163-1-xiyou.wangcong@gmail.com> References: <20190321061208.16163-1-xiyou.wangcong@gmail.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Similarly, we need to check skb->data boundary for HCI_EV_LE_META event too. Note, hci_le_adv_report_evt() and hci_le_ext_adv_report_evt() are slightly complicated, as they read the length of the field from the packet too. Cc: Marcel Holtmann Cc: Johan Hedberg Signed-off-by: Cong Wang --- net/bluetooth/hci_event.c | 107 +++++++++++++++++++++++++++++++------- 1 file changed, 87 insertions(+), 20 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 2fef70c0bffe..31aef14dd838 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -5147,7 +5147,11 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_conn_complete *ev = (void *) skb->data; + struct hci_ev_le_conn_complete *ev; + + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; BT_DBG("%s status 0x%2.2x", hdev->name, ev->status); @@ -5161,7 +5165,11 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_enh_conn_complete *ev = (void *) skb->data; + struct hci_ev_le_enh_conn_complete *ev; + + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; BT_DBG("%s status 0x%2.2x", hdev->name, ev->status); @@ -5174,9 +5182,13 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev, static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_evt_le_ext_adv_set_term *ev = (void *) skb->data; + struct hci_evt_le_ext_adv_set_term *ev; struct hci_conn *conn; + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; + BT_DBG("%s status 0x%2.2x", hdev->name, ev->status); if (ev->status) @@ -5203,9 +5215,13 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, struct sk_buff *skb) static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_conn_update_complete *ev = (void *) skb->data; + struct hci_ev_le_conn_update_complete *ev; struct hci_conn *conn; + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; + BT_DBG("%s status 0x%2.2x", hdev->name, ev->status); if (ev->status) @@ -5511,15 +5527,29 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr, static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) { - u8 num_reports = skb->data[0]; - void *ptr = &skb->data[1]; + unsigned int len; + u8 num_reports; + + if (unlikely(!pskb_may_pull(skb, 1))) + return; + num_reports = skb->data[0]; + len = 1; hci_dev_lock(hdev); while (num_reports--) { - struct hci_ev_le_advertising_info *ev = ptr; + struct hci_ev_le_advertising_info *ev; + u8 ev_len; s8 rssi; + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev)))) + break; + ev = (void *)skb->data + len; + ev_len = ev->length + 1; + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev) + ev_len))) + break; + ev = (void *)skb->data + len; + if (ev->length <= HCI_MAX_AD_LENGTH) { rssi = ev->data[ev->length]; process_adv_report(hdev, ev->evt_type, &ev->bdaddr, @@ -5529,7 +5559,7 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) bt_dev_err(hdev, "Dropping invalid advertising data"); } - ptr += sizeof(*ev) + ev->length + 1; + len += sizeof(*ev) + ev_len; } hci_dev_unlock(hdev); @@ -5583,15 +5613,29 @@ static u8 ext_evt_type_to_legacy(u16 evt_type) static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) { - u8 num_reports = skb->data[0]; - void *ptr = &skb->data[1]; + unsigned int len; + u8 num_reports; + + if (unlikely(!pskb_may_pull(skb, 1))) + return; + num_reports = skb->data[0]; + len = 1; hci_dev_lock(hdev); while (num_reports--) { - struct hci_ev_le_ext_adv_report *ev = ptr; + struct hci_ev_le_ext_adv_report *ev; u8 legacy_evt_type; u16 evt_type; + u8 ev_len; + + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev)))) + break; + ev = (void *)skb->data + len; + ev_len = ev->length + 1; + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev) + ev_len))) + break; + ev = (void *)skb->data + len; evt_type = __le16_to_cpu(ev->evt_type); legacy_evt_type = ext_evt_type_to_legacy(evt_type); @@ -5601,7 +5645,7 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) ev->data, ev->length); } - ptr += sizeof(*ev) + ev->length + 1; + len += sizeof(*ev) + ev_len; } hci_dev_unlock(hdev); @@ -5610,9 +5654,13 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) static void hci_le_remote_feat_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_remote_feat_complete *ev = (void *)skb->data; + struct hci_ev_le_remote_feat_complete *ev; struct hci_conn *conn; + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; + BT_DBG("%s status 0x%2.2x", hdev->name, ev->status); hci_dev_lock(hdev); @@ -5651,12 +5699,16 @@ static void hci_le_remote_feat_complete_evt(struct hci_dev *hdev, static void hci_le_ltk_request_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_ltk_req *ev = (void *) skb->data; + struct hci_ev_le_ltk_req *ev; struct hci_cp_le_ltk_reply cp; struct hci_cp_le_ltk_neg_reply neg; struct hci_conn *conn; struct smp_ltk *ltk; + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; + BT_DBG("%s handle 0x%4.4x", hdev->name, __le16_to_cpu(ev->handle)); hci_dev_lock(hdev); @@ -5728,11 +5780,15 @@ static void send_conn_param_neg_reply(struct hci_dev *hdev, u16 handle, static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_remote_conn_param_req *ev = (void *) skb->data; + struct hci_ev_le_remote_conn_param_req *ev; struct hci_cp_le_conn_param_req_reply cp; struct hci_conn *hcon; u16 handle, min, max, latency, timeout; + if (unlikely(!pskb_may_pull(skb, sizeof(*ev)))) + return; + ev = (void *)skb->data; + handle = le16_to_cpu(ev->handle); min = le16_to_cpu(ev->interval_min); max = le16_to_cpu(ev->interval_max); @@ -5786,19 +5842,27 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev, static void hci_le_direct_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) { - u8 num_reports = skb->data[0]; - void *ptr = &skb->data[1]; + unsigned int len; + u8 num_reports; + + if (unlikely(!pskb_may_pull(skb, 1))) + return; + num_reports = skb->data[0]; + len = 1; hci_dev_lock(hdev); while (num_reports--) { - struct hci_ev_le_direct_adv_info *ev = ptr; + struct hci_ev_le_direct_adv_info *ev; + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev)))) + break; + ev = (void *)skb->data + len; process_adv_report(hdev, ev->evt_type, &ev->bdaddr, ev->bdaddr_type, &ev->direct_addr, ev->direct_addr_type, ev->rssi, NULL, 0); - ptr += sizeof(*ev); + len += sizeof(*ev); } hci_dev_unlock(hdev); @@ -5806,8 +5870,11 @@ static void hci_le_direct_adv_report_evt(struct hci_dev *hdev, static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb) { - struct hci_ev_le_meta *le_ev = (void *) skb->data; + struct hci_ev_le_meta *le_ev; + if (unlikely(!pskb_may_pull(skb, sizeof(*le_ev)))) + return; + le_ev = (void *)skb->data; skb_pull(skb, sizeof(*le_ev)); switch (le_ev->subevent) {