* [Patch net v2 0/3] bluetooth: validate packet boundary carefully
@ 2019-03-21 6:12 Cong Wang
2019-03-21 6:12 ` [Patch net v2 1/3] bluetooth: validate HCI_EVENT_PKT packet carefully Cong Wang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Cong Wang @ 2019-03-21 6:12 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
This patchset fixes the out-of-bound access in bluetooth stack.
Although it is large, all of them follow the same pattern, so
it should not be hard to review. I try to group them as much as
I can.
Please check each patch for details.
Cong Wang (3):
bluetooth: validate HCI_EVENT_PKT packet carefully
bluetooth: validate HCI_EV_LE_META packet carefully
bluetooth: validate HCI_EV_CMD_COMPLETE packet carefully
net/bluetooth/hci_event.c | 627 +++++++++++++++++++++++++++++++-------
1 file changed, 520 insertions(+), 107 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch net v2 1/3] bluetooth: validate HCI_EVENT_PKT packet carefully
2019-03-21 6:12 [Patch net v2 0/3] bluetooth: validate packet boundary carefully Cong Wang
@ 2019-03-21 6:12 ` Cong Wang
2019-03-21 6:12 ` [Patch net v2 2/3] bluetooth: validate HCI_EV_LE_META " Cong Wang
2019-03-21 6:12 ` [Patch net v2 3/3] bluetooth: validate HCI_EV_CMD_COMPLETE " Cong Wang
2 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-03-21 6:12 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, syzbot+cec7a50c412a2c03f8f5,
syzbot+660883c56e2fa65d4497, Marcel Holtmann, Johan Hedberg
hci_event_packet() blindly assumes all packets are sane, at least
for packets allocated via vhci_get_user() path this is not true.
We have to check if we access skb data out-of-bound with
pskb_may_pull() before each skb->data dereference on RX path.
Reported-and-tested-by: syzbot+cec7a50c412a2c03f8f5@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+660883c56e2fa65d4497@syzkaller.appspotmail.com
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/bluetooth/hci_event.c | 262 +++++++++++++++++++++++++++++++-------
1 file changed, 218 insertions(+), 44 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 609fd6871c5a..2fef70c0bffe 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2331,10 +2331,13 @@ static void hci_cs_switch_role(struct hci_dev *hdev, u8 status)
static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- __u8 status = *((__u8 *) skb->data);
struct discovery_state *discov = &hdev->discovery;
struct inquiry_entry *e;
+ __u8 status;
+ if (unlikely(!pskb_may_pull(skb, 1)))
+ return;
+ status = *((__u8 *)skb->data);
BT_DBG("%s status 0x%2.2x", hdev->name, status);
hci_conn_check_pending(hdev);
@@ -2391,14 +2394,21 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct inquiry_data data;
- struct inquiry_info *info = (void *) (skb->data + 1);
- int num_rsp = *((__u8 *) skb->data);
+ struct inquiry_info *info;
+ int num_rsp;
BT_DBG("%s num_rsp %d", hdev->name, num_rsp);
+ if (unlikely(!pskb_may_pull(skb, 1)))
+ return;
+ num_rsp = *((__u8 *)skb->data);
if (!num_rsp)
return;
+ if (unlikely(!pskb_may_pull(skb, 1 + num_rsp * sizeof(*info))))
+ return;
+ info = (void *)(skb->data + 1);
+
if (hci_dev_test_flag(hdev, HCI_PERIODIC_INQ))
return;
@@ -2428,11 +2438,15 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_conn_complete *ev = (void *) skb->data;
+ struct hci_ev_conn_complete *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
@@ -2522,12 +2536,16 @@ static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr)
static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_conn_request *ev = (void *) skb->data;
+ struct hci_ev_conn_request *ev;
int mask = hdev->link_mode;
struct inquiry_entry *ie;
struct hci_conn *conn;
__u8 flags = 0;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
ev->link_type);
@@ -2633,13 +2651,17 @@ static u8 hci_to_mgmt_reason(u8 err)
static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_disconn_complete *ev = (void *) skb->data;
+ struct hci_ev_disconn_complete *ev;
u8 reason;
struct hci_conn_params *params;
struct hci_conn *conn;
bool mgmt_connected;
u8 type;
+ 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);
@@ -2717,9 +2739,13 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_auth_complete *ev = (void *) skb->data;
+ struct hci_ev_auth_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);
@@ -2787,11 +2813,15 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_remote_name *ev = (void *) skb->data;
+ struct hci_ev_remote_name *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_conn_check_pending(hdev);
hci_dev_lock(hdev);
@@ -2885,9 +2915,13 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_encrypt_change *ev = (void *) skb->data;
+ struct hci_ev_encrypt_change *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);
@@ -2992,9 +3026,13 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_change_link_key_complete *ev = (void *) skb->data;
+ struct hci_ev_change_link_key_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);
@@ -3015,9 +3053,13 @@ static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
static void hci_remote_features_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_remote_features *ev = (void *) skb->data;
+ struct hci_ev_remote_features *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);
@@ -3413,7 +3455,11 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_req_complete_t *req_complete,
hci_req_complete_skb_t *req_complete_skb)
{
- struct hci_ev_cmd_status *ev = (void *) skb->data;
+ struct hci_ev_cmd_status *ev;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
skb_pull(skb, sizeof(*ev));
@@ -3517,7 +3563,11 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
static void hci_hardware_error_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_hardware_error *ev = (void *) skb->data;
+ struct hci_ev_hardware_error *ev;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
hdev->hw_error_code = ev->code;
@@ -3526,9 +3576,13 @@ static void hci_hardware_error_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_role_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_role_change *ev = (void *) skb->data;
+ struct hci_ev_role_change *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);
@@ -3687,9 +3741,13 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_mode_change *ev = (void *) skb->data;
+ struct hci_ev_mode_change *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);
@@ -3715,11 +3773,15 @@ static void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_pin_code_req *ev = (void *) skb->data;
+ struct hci_ev_pin_code_req *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -3785,13 +3847,17 @@ static void conn_set_key(struct hci_conn *conn, u8 key_type, u8 pin_len)
static void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_link_key_req *ev = (void *) skb->data;
+ struct hci_ev_link_key_req *ev;
struct hci_cp_link_key_reply cp;
struct hci_conn *conn;
struct link_key *key;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
if (!hci_dev_test_flag(hdev, HCI_MGMT))
return;
@@ -3845,7 +3911,7 @@ static void hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_link_key_notify *ev = (void *) skb->data;
+ struct hci_ev_link_key_notify *ev;
struct hci_conn *conn;
struct link_key *key;
bool persistent;
@@ -3853,6 +3919,10 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -3905,9 +3975,13 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_clock_offset_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_clock_offset *ev = (void *) skb->data;
+ struct hci_ev_clock_offset *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);
@@ -3928,9 +4002,13 @@ static void hci_clock_offset_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_pkt_type_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_pkt_type_change *ev = (void *) skb->data;
+ struct hci_ev_pkt_type_change *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);
@@ -3944,11 +4022,15 @@ static void hci_pkt_type_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_pscan_rep_mode_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_pscan_rep_mode *ev = (void *) skb->data;
+ struct hci_ev_pscan_rep_mode *ev;
struct inquiry_entry *ie;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
ie = hci_inquiry_cache_lookup(hdev, &ev->bdaddr);
@@ -3964,8 +4046,12 @@ static void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
struct inquiry_data data;
- int num_rsp = *((__u8 *) skb->data);
+ int num_rsp;
+
+ if (unlikely(!pskb_may_pull(skb, 1)))
+ return;
+ num_rsp = *((__u8 *)skb->data);
BT_DBG("%s num_rsp %d", hdev->name, num_rsp);
if (!num_rsp)
@@ -3978,6 +4064,9 @@ static void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev,
if ((skb->len - 1) / num_rsp != sizeof(struct inquiry_info_with_rssi)) {
struct inquiry_info_with_rssi_and_pscan_mode *info;
+
+ if (unlikely(!pskb_may_pull(skb, 1 + num_rsp * sizeof(*info))))
+ goto unlock;
info = (void *) (skb->data + 1);
for (; num_rsp; num_rsp--, info++) {
@@ -3999,7 +4088,11 @@ static void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev,
flags, NULL, 0, NULL, 0);
}
} else {
- struct inquiry_info_with_rssi *info = (void *) (skb->data + 1);
+ struct inquiry_info_with_rssi *info;
+
+ if (unlikely(!pskb_may_pull(skb, 1 + num_rsp * sizeof(*info))))
+ goto unlock;
+ info = (void *)(skb->data + 1);
for (; num_rsp; num_rsp--, info++) {
u32 flags;
@@ -4021,17 +4114,22 @@ static void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev,
}
}
+unlock:
hci_dev_unlock(hdev);
}
static void hci_remote_ext_features_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_remote_ext_features *ev = (void *) skb->data;
+ struct hci_ev_remote_ext_features *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
@@ -4091,9 +4189,13 @@ static void hci_remote_ext_features_evt(struct hci_dev *hdev,
static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_sync_conn_complete *ev = (void *) skb->data;
+ struct hci_ev_sync_conn_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);
@@ -4176,14 +4278,21 @@ static void hci_extended_inquiry_result_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
struct inquiry_data data;
- struct extended_inquiry_info *info = (void *) (skb->data + 1);
- int num_rsp = *((__u8 *) skb->data);
+ struct extended_inquiry_info *info;
+ int num_rsp;
size_t eir_len;
+ if (unlikely(!pskb_may_pull(skb, 1)))
+ return;
+ num_rsp = *((__u8 *)skb->data);
+
BT_DBG("%s num_rsp %d", hdev->name, num_rsp);
if (!num_rsp)
return;
+ if (unlikely(!pskb_may_pull(skb, 1 + num_rsp * sizeof(*info))))
+ return;
+ info = (void *)(skb->data + 1);
if (hci_dev_test_flag(hdev, HCI_PERIODIC_INQ))
return;
@@ -4225,9 +4334,13 @@ static void hci_extended_inquiry_result_evt(struct hci_dev *hdev,
static void hci_key_refresh_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_key_refresh_complete *ev = (void *) skb->data;
+ struct hci_ev_key_refresh_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 handle 0x%4.4x", hdev->name, ev->status,
__le16_to_cpu(ev->handle));
@@ -4334,11 +4447,15 @@ static u8 bredr_oob_data_present(struct hci_conn *conn)
static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_io_capa_request *ev = (void *) skb->data;
+ struct hci_ev_io_capa_request *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -4403,11 +4520,15 @@ static void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_io_capa_reply_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_io_capa_reply *ev = (void *) skb->data;
+ struct hci_ev_io_capa_reply *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -4424,12 +4545,16 @@ static void hci_io_capa_reply_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_user_confirm_request_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_user_confirm_req *ev = (void *) skb->data;
+ struct hci_ev_user_confirm_req *ev;
int loc_mitm, rem_mitm, confirm_hint = 0;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
if (!hci_dev_test_flag(hdev, HCI_MGMT))
@@ -4499,10 +4624,13 @@ static void hci_user_confirm_request_evt(struct hci_dev *hdev,
static void hci_user_passkey_request_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_user_passkey_req *ev = (void *) skb->data;
+ struct hci_ev_user_passkey_req *ev;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
if (hci_dev_test_flag(hdev, HCI_MGMT))
mgmt_user_passkey_request(hdev, &ev->bdaddr, ACL_LINK, 0);
}
@@ -4510,11 +4638,15 @@ static void hci_user_passkey_request_evt(struct hci_dev *hdev,
static void hci_user_passkey_notify_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_user_passkey_notify *ev = (void *) skb->data;
+ struct hci_ev_user_passkey_notify *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
if (!conn)
return;
@@ -4530,11 +4662,15 @@ static void hci_user_passkey_notify_evt(struct hci_dev *hdev,
static void hci_keypress_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_keypress_notify *ev = (void *) skb->data;
+ struct hci_ev_keypress_notify *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
if (!conn)
return;
@@ -4569,11 +4705,15 @@ static void hci_keypress_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_simple_pair_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_simple_pair_complete *ev = (void *) skb->data;
+ struct hci_ev_simple_pair_complete *ev;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -4600,12 +4740,16 @@ static void hci_simple_pair_complete_evt(struct hci_dev *hdev,
static void hci_remote_host_features_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_remote_host_features *ev = (void *) skb->data;
+ struct hci_ev_remote_host_features *ev;
struct inquiry_entry *ie;
struct hci_conn *conn;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
@@ -4622,11 +4766,15 @@ static void hci_remote_host_features_evt(struct hci_dev *hdev,
static void hci_remote_oob_data_request_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_remote_oob_data_request *ev = (void *) skb->data;
+ struct hci_ev_remote_oob_data_request *ev;
struct oob_data *data;
BT_DBG("%s", hdev->name);
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
hci_dev_lock(hdev);
if (!hci_dev_test_flag(hdev, HCI_MGMT))
@@ -4676,9 +4824,13 @@ static void hci_remote_oob_data_request_evt(struct hci_dev *hdev,
#if IS_ENABLED(CONFIG_BT_HS)
static void hci_chan_selected_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_channel_selected *ev = (void *)skb->data;
+ struct hci_ev_channel_selected *ev;
struct hci_conn *hcon;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
BT_DBG("%s handle 0x%2.2x", hdev->name, ev->phy_handle);
skb_pull(skb, sizeof(*ev));
@@ -4693,9 +4845,13 @@ static void hci_chan_selected_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_phy_link_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_phy_link_complete *ev = (void *) skb->data;
+ struct hci_ev_phy_link_complete *ev;
struct hci_conn *hcon, *bredr_hcon;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
BT_DBG("%s handle 0x%2.2x status 0x%2.2x", hdev->name, ev->phy_handle,
ev->status);
@@ -4732,11 +4888,15 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_ev_logical_link_complete *ev = (void *) skb->data;
+ struct hci_ev_logical_link_complete *ev;
struct hci_conn *hcon;
struct hci_chan *hchan;
struct amp_mgr *mgr;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
BT_DBG("%s log_handle 0x%4.4x phy_handle 0x%2.2x status 0x%2.2x",
hdev->name, le16_to_cpu(ev->handle), ev->phy_handle,
ev->status);
@@ -4771,9 +4931,13 @@ static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_disconn_logical_link_complete *ev = (void *) skb->data;
+ struct hci_ev_disconn_logical_link_complete *ev;
struct hci_chan *hchan;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev))))
+ return;
+ ev = (void *)skb->data;
+
BT_DBG("%s log handle 0x%4.4x status 0x%2.2x", hdev->name,
le16_to_cpu(ev->handle), ev->status);
@@ -4795,9 +4959,13 @@ static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
static void hci_disconn_phylink_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_ev_disconn_phy_link_complete *ev = (void *) skb->data;
+ struct hci_ev_disconn_phy_link_complete *ev;
struct hci_conn *hcon;
+ 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)
@@ -5742,13 +5910,18 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, u16 opcode,
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_event_hdr *hdr = (void *) skb->data;
+ struct hci_event_hdr *hdr;
hci_req_complete_t req_complete = NULL;
hci_req_complete_skb_t req_complete_skb = NULL;
struct sk_buff *orig_skb = NULL;
- u8 status = 0, event = hdr->evt, req_evt = 0;
+ u8 status = 0, event, req_evt = 0;
u16 opcode = HCI_OP_NOP;
+ if (unlikely(!pskb_may_pull(skb, HCI_EVENT_HDR_SIZE)))
+ goto free;
+ hdr = (void *)skb->data;
+ event = hdr->evt;
+
if (hdev->sent_cmd && bt_cb(hdev->sent_cmd)->hci.req_event == event) {
struct hci_command_hdr *cmd_hdr = (void *) hdev->sent_cmd->data;
opcode = __le16_to_cpu(cmd_hdr->opcode);
@@ -5960,6 +6133,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
req_complete_skb(hdev, status, opcode, orig_skb);
}
+free:
kfree_skb(orig_skb);
kfree_skb(skb);
hdev->stat.evt_rx++;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net v2 2/3] bluetooth: validate HCI_EV_LE_META packet carefully
2019-03-21 6:12 [Patch net v2 0/3] bluetooth: validate packet boundary carefully Cong Wang
2019-03-21 6:12 ` [Patch net v2 1/3] bluetooth: validate HCI_EVENT_PKT packet carefully Cong Wang
@ 2019-03-21 6:12 ` Cong Wang
2019-03-21 6:12 ` [Patch net v2 3/3] bluetooth: validate HCI_EV_CMD_COMPLETE " Cong Wang
2 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-03-21 6:12 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Marcel Holtmann, Johan Hedberg
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 <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
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) {
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net v2 3/3] bluetooth: validate HCI_EV_CMD_COMPLETE packet carefully
2019-03-21 6:12 [Patch net v2 0/3] bluetooth: validate packet boundary carefully Cong Wang
2019-03-21 6:12 ` [Patch net v2 1/3] bluetooth: validate HCI_EVENT_PKT packet carefully Cong Wang
2019-03-21 6:12 ` [Patch net v2 2/3] bluetooth: validate HCI_EV_LE_META " Cong Wang
@ 2019-03-21 6:12 ` Cong Wang
2 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-03-21 6:12 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Marcel Holtmann, Johan Hedberg
Similarly, we need to check skb->data boundary for
HCI_EV_CMD_COMPLETE event too.
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/bluetooth/hci_event.c | 258 +++++++++++++++++++++++++++++++-------
1 file changed, 215 insertions(+), 43 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 31aef14dd838..63e14c49f617 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -100,9 +100,13 @@ static void hci_cc_remote_name_req_cancel(struct hci_dev *hdev,
static void hci_cc_role_discovery(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_role_discovery *rp = (void *) skb->data;
+ struct hci_rp_role_discovery *rp;
struct hci_conn *conn;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -119,9 +123,13 @@ static void hci_cc_role_discovery(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_link_policy(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_link_policy *rp = (void *) skb->data;
+ struct hci_rp_read_link_policy *rp;
struct hci_conn *conn;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -138,10 +146,14 @@ static void hci_cc_read_link_policy(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_write_link_policy(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_write_link_policy *rp = (void *) skb->data;
+ struct hci_rp_write_link_policy *rp;
struct hci_conn *conn;
void *sent;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -163,7 +175,11 @@ static void hci_cc_write_link_policy(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_def_link_policy(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_def_link_policy *rp = (void *) skb->data;
+ struct hci_rp_read_def_link_policy *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -227,9 +243,13 @@ static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_stored_link_key(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_stored_link_key *rp = (void *)skb->data;
+ struct hci_rp_read_stored_link_key *rp;
struct hci_cp_read_stored_link_key *sent;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
sent = hci_sent_cmd_data(hdev, HCI_OP_READ_STORED_LINK_KEY);
@@ -245,7 +265,11 @@ static void hci_cc_read_stored_link_key(struct hci_dev *hdev,
static void hci_cc_delete_stored_link_key(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_delete_stored_link_key *rp = (void *)skb->data;
+ struct hci_rp_delete_stored_link_key *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -281,7 +305,11 @@ static void hci_cc_write_local_name(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_local_name(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_local_name *rp = (void *) skb->data;
+ struct hci_rp_read_local_name *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -381,7 +409,11 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_class_of_dev *rp = (void *) skb->data;
+ struct hci_rp_read_class_of_dev *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -418,9 +450,13 @@ static void hci_cc_write_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_voice_setting(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_voice_setting *rp = (void *) skb->data;
+ struct hci_rp_read_voice_setting *rp;
__u16 setting;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -471,7 +507,11 @@ static void hci_cc_write_voice_setting(struct hci_dev *hdev,
static void hci_cc_read_num_supported_iac(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_num_supported_iac *rp = (void *) skb->data;
+ struct hci_rp_read_num_supported_iac *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -547,7 +587,11 @@ static void hci_cc_write_sc_support(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_local_version *rp = (void *) skb->data;
+ struct hci_rp_read_local_version *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -567,7 +611,11 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_local_commands(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_commands *rp = (void *) skb->data;
+ struct hci_rp_read_local_commands *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -582,7 +630,11 @@ static void hci_cc_read_local_commands(struct hci_dev *hdev,
static void hci_cc_read_local_features(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_features *rp = (void *) skb->data;
+ struct hci_rp_read_local_features *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -632,7 +684,11 @@ static void hci_cc_read_local_features(struct hci_dev *hdev,
static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_ext_features *rp = (void *) skb->data;
+ struct hci_rp_read_local_ext_features *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -649,7 +705,11 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_flow_control_mode *rp = (void *) skb->data;
+ struct hci_rp_read_flow_control_mode *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -661,7 +721,11 @@ static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_buffer_size *rp = (void *) skb->data;
+ struct hci_rp_read_buffer_size *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -687,7 +751,11 @@ static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_bd_addr *rp = (void *) skb->data;
+ struct hci_rp_read_bd_addr *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -704,7 +772,11 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_page_scan_activity(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_page_scan_activity *rp = (void *) skb->data;
+ struct hci_rp_read_page_scan_activity *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -739,7 +811,11 @@ static void hci_cc_write_page_scan_activity(struct hci_dev *hdev,
static void hci_cc_read_page_scan_type(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_page_scan_type *rp = (void *) skb->data;
+ struct hci_rp_read_page_scan_type *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -769,7 +845,11 @@ static void hci_cc_write_page_scan_type(struct hci_dev *hdev,
static void hci_cc_read_data_block_size(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_data_block_size *rp = (void *) skb->data;
+ struct hci_rp_read_data_block_size *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -824,7 +904,11 @@ static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_local_amp_info(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_amp_info *rp = (void *) skb->data;
+ struct hci_rp_read_local_amp_info *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -846,7 +930,11 @@ static void hci_cc_read_local_amp_info(struct hci_dev *hdev,
static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_inq_rsp_tx_power *rp = (void *) skb->data;
+ struct hci_rp_read_inq_rsp_tx_power *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -858,10 +946,14 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_pin_code_reply *rp = (void *) skb->data;
+ struct hci_rp_pin_code_reply *rp;
struct hci_cp_pin_code_reply *cp;
struct hci_conn *conn;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
hci_dev_lock(hdev);
@@ -886,7 +978,11 @@ static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_pin_code_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_pin_code_neg_reply *rp = (void *) skb->data;
+ struct hci_rp_pin_code_neg_reply *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -902,7 +998,11 @@ static void hci_cc_pin_code_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_buffer_size *rp = (void *) skb->data;
+ struct hci_rp_le_read_buffer_size *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -920,7 +1020,11 @@ static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
static void hci_cc_le_read_local_features(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_local_features *rp = (void *) skb->data;
+ struct hci_rp_le_read_local_features *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -933,7 +1037,11 @@ static void hci_cc_le_read_local_features(struct hci_dev *hdev,
static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_adv_tx_power *rp = (void *) skb->data;
+ struct hci_rp_le_read_adv_tx_power *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -945,7 +1053,11 @@ static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
static void hci_cc_user_confirm_reply(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_user_confirm_reply *rp = (void *) skb->data;
+ struct hci_rp_user_confirm_reply *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -961,7 +1073,11 @@ static void hci_cc_user_confirm_reply(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_user_confirm_neg_reply(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_user_confirm_reply *rp = (void *) skb->data;
+ struct hci_rp_user_confirm_reply *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -976,7 +1092,11 @@ static void hci_cc_user_confirm_neg_reply(struct hci_dev *hdev,
static void hci_cc_user_passkey_reply(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_user_confirm_reply *rp = (void *) skb->data;
+ struct hci_rp_user_confirm_reply *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -992,7 +1112,11 @@ static void hci_cc_user_passkey_reply(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_user_passkey_neg_reply(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_user_confirm_reply *rp = (void *) skb->data;
+ struct hci_rp_user_confirm_reply *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -1008,7 +1132,11 @@ static void hci_cc_user_passkey_neg_reply(struct hci_dev *hdev,
static void hci_cc_read_local_oob_data(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_oob_data *rp = (void *) skb->data;
+ struct hci_rp_read_local_oob_data *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
}
@@ -1016,7 +1144,11 @@ static void hci_cc_read_local_oob_data(struct hci_dev *hdev,
static void hci_cc_read_local_oob_ext_data(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_read_local_oob_ext_data *rp = (void *) skb->data;
+ struct hci_rp_read_local_oob_ext_data *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
}
@@ -1333,7 +1465,11 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
+ struct hci_rp_le_read_num_supported_adv_sets *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
rp->num_of_sets);
@@ -1347,7 +1483,11 @@ static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_white_list_size *rp = (void *) skb->data;
+ struct hci_rp_le_read_white_list_size *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x size %u", hdev->name, rp->status, rp->size);
@@ -1411,7 +1551,11 @@ static void hci_cc_le_del_from_white_list(struct hci_dev *hdev,
static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_supported_states *rp = (void *) skb->data;
+ struct hci_rp_le_read_supported_states *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -1424,7 +1568,11 @@ static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
static void hci_cc_le_read_def_data_len(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_def_data_len *rp = (void *) skb->data;
+ struct hci_rp_le_read_def_data_len *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -1509,7 +1657,11 @@ static void hci_cc_le_clear_resolv_list(struct hci_dev *hdev,
static void hci_cc_le_read_resolv_list_size(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_resolv_list_size *rp = (void *) skb->data;
+ struct hci_rp_le_read_resolv_list_size *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x size %u", hdev->name, rp->status, rp->size);
@@ -1546,7 +1698,11 @@ static void hci_cc_le_set_addr_resolution_enable(struct hci_dev *hdev,
static void hci_cc_le_read_max_data_len(struct hci_dev *hdev,
struct sk_buff *skb)
{
- struct hci_rp_le_read_max_data_len *rp = (void *) skb->data;
+ struct hci_rp_le_read_max_data_len *rp;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
@@ -1614,10 +1770,14 @@ static void hci_cc_set_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_set_ext_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_le_set_ext_adv_params *rp = (void *) skb->data;
+ struct hci_rp_le_set_ext_adv_params *rp;
struct hci_cp_le_set_ext_adv_params *cp;
struct adv_info *adv_instance;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -1645,9 +1805,13 @@ static void hci_cc_set_ext_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_rssi(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct hci_rp_read_rssi *rp = (void *) skb->data;
+ struct hci_rp_read_rssi *rp;
struct hci_conn *conn;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -1665,9 +1829,13 @@ static void hci_cc_read_rssi(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_cc_read_tx_power(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_cp_read_tx_power *sent;
- struct hci_rp_read_tx_power *rp = (void *) skb->data;
+ struct hci_rp_read_tx_power *rp;
struct hci_conn *conn;
+ if (unlikely(!pskb_may_pull(skb, sizeof(*rp))))
+ return;
+ rp = (void *)skb->data;
+
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
if (rp->status)
@@ -3108,7 +3276,11 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_req_complete_t *req_complete,
hci_req_complete_skb_t *req_complete_skb)
{
- struct hci_ev_cmd_complete *ev = (void *) skb->data;
+ struct hci_ev_cmd_complete *ev;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(*ev) + 1)))
+ return;
+ ev = (void *)skb->data;
*opcode = __le16_to_cpu(ev->opcode);
*status = skb->data[sizeof(*ev)];
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net v2 0/3] bluetooth: validate packet boundary carefully
@ 2019-04-03 23:08 Cong Wang
0 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-04-03 23:08 UTC (permalink / raw)
To: netdev; +Cc: linux-bluetooth, Cong Wang
This patchset fixes the out-of-bound access in bluetooth stack.
Although it is large, all of them follow the same pattern, so
it should not be hard to review. I try to group them as much as
I can. This patchset should cover Dan's patch too.
Please check each patch for details.
(Resending v2 as it is lost in netdev mailing list.)
---
Cong Wang (3):
bluetooth: validate HCI_EVENT_PKT packet carefully
bluetooth: validate HCI_EV_LE_META packet carefully
bluetooth: validate HCI_EV_CMD_COMPLETE packet carefully
net/bluetooth/hci_event.c | 627 +++++++++++++++++++++++++++++++-------
1 file changed, 520 insertions(+), 107 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-03 23:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 6:12 [Patch net v2 0/3] bluetooth: validate packet boundary carefully Cong Wang
2019-03-21 6:12 ` [Patch net v2 1/3] bluetooth: validate HCI_EVENT_PKT packet carefully Cong Wang
2019-03-21 6:12 ` [Patch net v2 2/3] bluetooth: validate HCI_EV_LE_META " Cong Wang
2019-03-21 6:12 ` [Patch net v2 3/3] bluetooth: validate HCI_EV_CMD_COMPLETE " Cong Wang
2019-04-03 23:08 [Patch net v2 0/3] bluetooth: validate packet boundary carefully Cong Wang
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.