Commit 2615fd9a authored by Luiz Augusto von Dentz's avatar Luiz Augusto von Dentz
Browse files

Bluetooth: hci_sync: Fix overwriting request callback 



In a few cases the stack may generate commands as responses to events
which would happen to overwrite the sent_cmd, so this attempts to store
the request in req_skb so even if sent_cmd is replaced with a new
command the pending request will remain in stored in req_skb.

Fixes: 6a98e383 ("Bluetooth: Add helper for serialized HCI command execution")
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent 22cbf4f8

include/net/bluetooth/hci_core.h

+1 −0
Original line number Diff line number Diff line
@@ -552,6 +552,7 @@ struct hci_dev { 
	__u32			req_status;

	__u32			req_result;

	struct sk_buff		*req_skb;

	struct sk_buff		*req_rsp;



	void			*smp_data;

	void			*smp_bredr_data;

net/bluetooth/hci_conn.c

+1 −1
Original line number Diff line number Diff line
@@ -2918,7 +2918,7 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) 
		case HCI_EV_LE_CONN_COMPLETE:

		case HCI_EV_LE_ENHANCED_CONN_COMPLETE:

		case HCI_EVT_LE_CIS_ESTABLISHED:

			hci_cmd_sync_cancel(hdev, -ECANCELED);

			hci_cmd_sync_cancel(hdev, ECANCELED);

			break;

		}

	/* Cancel connect attempt if still queued/pending */

net/bluetooth/hci_core.c

+32 −14
Original line number Diff line number Diff line
@@ -1522,8 +1522,8 @@ static void hci_cmd_timeout(struct work_struct *work) 
	struct hci_dev *hdev = container_of(work, struct hci_dev,

					    cmd_timer.work);



	if (hdev->sent_cmd) {

		u16 opcode = hci_skb_opcode(hdev->sent_cmd);

	if (hdev->req_skb) {

		u16 opcode = hci_skb_opcode(hdev->req_skb);



		bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
@@ -2828,6 +2828,7 @@ void hci_release_dev(struct hci_dev *hdev) 
	ida_destroy(&hdev->unset_handle_ida);

	ida_free(&hci_index_ida, hdev->id);

	kfree_skb(hdev->sent_cmd);

	kfree_skb(hdev->req_skb);

	kfree_skb(hdev->recv_event);

	kfree(hdev);

}
@@ -3157,21 +3158,33 @@ int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen, 
EXPORT_SYMBOL(__hci_cmd_send);



/* Get data from the previously sent command */

void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)

static void *hci_cmd_data(struct sk_buff *skb, __u16 opcode)

{

	struct hci_command_hdr *hdr;



	if (!hdev->sent_cmd)

	if (!skb || skb->len < HCI_COMMAND_HDR_SIZE)

		return NULL;



	hdr = (void *) hdev->sent_cmd->data;

	hdr = (void *)skb->data;



	if (hdr->opcode != cpu_to_le16(opcode))

		return NULL;



	BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);

	return skb->data + HCI_COMMAND_HDR_SIZE;

}



/* Get data from the previously sent command */

void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)

{

	void *data;



	/* Check if opcode matches last sent command */

	data = hci_cmd_data(hdev->sent_cmd, opcode);

	if (!data)

		/* Check if opcode matches last request */

		data = hci_cmd_data(hdev->req_skb, opcode);



	return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE;

	return data;

}



/* Get data from last received event */
@@ -4072,17 +4085,19 @@ void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status, 
	if (!status && !hci_req_is_complete(hdev))

		return;



	skb = hdev->req_skb;



	/* If this was the last command in a request the complete

	 * callback would be found in hdev->sent_cmd instead of the

	 * callback would be found in hdev->req_skb instead of the

	 * command queue (hdev->cmd_q).

	 */

	if (bt_cb(hdev->sent_cmd)->hci.req_flags & HCI_REQ_SKB) {

		*req_complete_skb = bt_cb(hdev->sent_cmd)->hci.req_complete_skb;

	if (skb && bt_cb(skb)->hci.req_flags & HCI_REQ_SKB) {

		*req_complete_skb = bt_cb(skb)->hci.req_complete_skb;

		return;

	}



	if (bt_cb(hdev->sent_cmd)->hci.req_complete) {

		*req_complete = bt_cb(hdev->sent_cmd)->hci.req_complete;

	if (skb && bt_cb(skb)->hci.req_complete) {

		*req_complete = bt_cb(skb)->hci.req_complete;

		return;

	}
@@ -4199,8 +4214,11 @@ static void hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb) 
		return;

	}



	if (hci_req_status_pend(hdev))

		hci_dev_set_flag(hdev, HCI_CMD_PENDING);

	if (hci_req_status_pend(hdev) &&

	    !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) {

		kfree_skb(hdev->req_skb);

		hdev->req_skb = skb_clone(skb, GFP_KERNEL);

	}



	atomic_dec(&hdev->cmd_cnt);

}

net/bluetooth/hci_event.c

+9 −9
Original line number Diff line number Diff line
@@ -4368,7 +4368,7 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, void *data, 
	 * (since for this kind of commands there will not be a command

	 * complete event).

	 */

	if (ev->status || (hdev->sent_cmd && !hci_skb_event(hdev->sent_cmd))) {

	if (ev->status || (hdev->req_skb && !hci_skb_event(hdev->req_skb))) {

		hci_req_cmd_complete(hdev, *opcode, ev->status, req_complete,

				     req_complete_skb);

		if (hci_dev_test_flag(hdev, HCI_CMD_PENDING)) {
@@ -7170,10 +7170,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, void *data, 
	bt_dev_dbg(hdev, "subevent 0x%2.2x", ev->subevent);



	/* Only match event if command OGF is for LE */

	if (hdev->sent_cmd &&

	    hci_opcode_ogf(hci_skb_opcode(hdev->sent_cmd)) == 0x08 &&

	    hci_skb_event(hdev->sent_cmd) == ev->subevent) {

		*opcode = hci_skb_opcode(hdev->sent_cmd);

	if (hdev->req_skb &&

	    hci_opcode_ogf(hci_skb_opcode(hdev->req_skb)) == 0x08 &&

	    hci_skb_event(hdev->req_skb) == ev->subevent) {

		*opcode = hci_skb_opcode(hdev->req_skb);

		hci_req_cmd_complete(hdev, *opcode, 0x00, req_complete,

				     req_complete_skb);

	}
@@ -7541,10 +7541,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb) 
	}



	/* Only match event if command OGF is not for LE */

	if (hdev->sent_cmd &&

	    hci_opcode_ogf(hci_skb_opcode(hdev->sent_cmd)) != 0x08 &&

	    hci_skb_event(hdev->sent_cmd) == event) {

		hci_req_cmd_complete(hdev, hci_skb_opcode(hdev->sent_cmd),

	if (hdev->req_skb &&

	    hci_opcode_ogf(hci_skb_opcode(hdev->req_skb)) != 0x08 &&

	    hci_skb_event(hdev->req_skb) == event) {

		hci_req_cmd_complete(hdev, hci_skb_opcode(hdev->req_skb),

				     status, &req_complete, &req_complete_skb);

		req_evt = event;

	}

net/bluetooth/hci_sync.c

+18 −3
Original line number Diff line number Diff line
@@ -32,6 +32,10 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, 
	hdev->req_result = result;

	hdev->req_status = HCI_REQ_DONE;



	/* Free the request command so it is not used as response */

	kfree_skb(hdev->req_skb);

	hdev->req_skb = NULL;



	if (skb) {

		struct sock *sk = hci_skb_sk(skb);
@@ -39,7 +43,7 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, 
		if (sk)

			sock_put(sk);



		hdev->req_skb = skb_get(skb);

		hdev->req_rsp = skb_get(skb);

	}



	wake_up_interruptible(&hdev->req_wait_q);
@@ -187,8 +191,8 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, 


	hdev->req_status = 0;

	hdev->req_result = 0;

	skb = hdev->req_skb;

	hdev->req_skb = NULL;

	skb = hdev->req_rsp;

	hdev->req_rsp = NULL;



	bt_dev_dbg(hdev, "end: err %d", err);
@@ -5021,6 +5025,11 @@ int hci_dev_open_sync(struct hci_dev *hdev) 
			hdev->sent_cmd = NULL;

		}



		if (hdev->req_skb) {

			kfree_skb(hdev->req_skb);

			hdev->req_skb = NULL;

		}



		clear_bit(HCI_RUNNING, &hdev->flags);

		hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
@@ -5181,6 +5190,12 @@ int hci_dev_close_sync(struct hci_dev *hdev) 
		hdev->sent_cmd = NULL;

	}



	/* Drop last request */

	if (hdev->req_skb) {

		kfree_skb(hdev->req_skb);

		hdev->req_skb = NULL;

	}



	clear_bit(HCI_RUNNING, &hdev->flags);

	hci_sock_dev_event(hdev, HCI_DEV_CLOSE);