linux-kernel - Re: [PATCH v5 3/4] Bluetooth: Allow driver specific cmd timeout handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Jan 2019 10:36:21 +0100
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Rajat Jain <rajatja@...gle.com>
Cc:     Johan Hedberg <johan.hedberg@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Dmitry Torokhov <dtor@...omium.org>,
        Alex Hung <alex.hung@...onical.com>,
        Bluez mailing list <linux-bluetooth@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-usb@...r.kernel.org, netdev <netdev@...r.kernel.org>,
        rajatxjain@...il.com, dtor@...gle.com,
        Raghuram Hegde <raghuram.hegde@...el.com>,
        chethan.tumkur.narayan@...el.com, sukumar.ghorai@...el.com
Subject: Re: [PATCH v5 3/4] Bluetooth: Allow driver specific cmd timeout
 handling

Hi Rajat,

> Add a hook to allow the BT driver to do device or command specific
> handling in case of timeouts. This is to be used by Intel driver to
> reset the device after certain number of timeouts.
> 
> Signed-off-by: Rajat Jain <rajatja@...gle.com>
> ---
> v5: Drop the quirk, and rename the hook function to cmd_timeout()
> v4: same as v1
> v3: same as v1
> v2: same as v1
> 
> include/net/bluetooth/hci_core.h |  1 +
> net/bluetooth/hci_core.c         | 10 ++++++++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e5ea633ea368..624d5f2b1f36 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -437,6 +437,7 @@ struct hci_dev {
> 	int (*post_init)(struct hci_dev *hdev);
> 	int (*set_diag)(struct hci_dev *hdev, bool enable);
> 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> +	void (*cmd_timeout)(struct hci_dev *hdev, struct hci_command_hdr *cmd);
> };
> 
> #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7352fe85674b..c6917f57581a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2568,16 +2568,22 @@ static void hci_cmd_timeout(struct work_struct *work)
> {
> 	struct hci_dev *hdev = container_of(work, struct hci_dev,
> 					    cmd_timer.work);
> +	struct hci_command_hdr *sent = NULL;
> 
> 	if (hdev->sent_cmd) {
> -		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> -		u16 opcode = __le16_to_cpu(sent->opcode);
> +		u16 opcode;
> +
> +		sent = (void *) hdev->sent_cmd->data;
> +		opcode = __le16_to_cpu(sent->opcode);
> 
> 		bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
> 	} else {
> 		bt_dev_err(hdev, "command tx timeout");
> 	}
> 
> +	if (hdev->cmd_timeout)
> +		hdev->cmd_timeout(hdev, sent);
> +

drop the sent parameter. You are not using it and if at some point in the future you might want to use it, then we add it and fix up all users.

And frankly, I would move the hdev->cmd_timeout call into the hdev->sent_cmd block since I do not even know if it makes sense to deal with the fallback case where hdev->sent_cmd is not available. Unless you have that kind of error case, but that is only possible if you have external injection of HCI commands.

Regards

Marcel