Re: [PATCH] Bluetooth: bring device down when closing HCI_CHANNEL_USER socket

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Simon,

On Fri, Aug 28, 2015, Simon Fels wrote:
> When a HCI_CHANNEL_USER socket is open and should be closed we first
> try to close the device which will fail as hci_dev_close checks for
> HCI_CHANNEL_USER being set and if it is it just fails to close the
> device. Clearing the HCI_CHANNEL_USER flag first before trying to
> close the device fixes this.
> 
> Signed-off-by: Simon Fels <simon.fels@canonical.com>
> ---
>  backports/net/bluetooth/hci_sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/backports/net/bluetooth/hci_sock.c b/backports/net/bluetooth/hci_sock.c
> index 9a2732f..c84c13e 100644
> --- a/backports/net/bluetooth/hci_sock.c
> +++ b/backports/net/bluetooth/hci_sock.c
> @@ -503,8 +503,8 @@ static int hci_sock_release(struct socket *sock)
>  
>  	if (hdev) {
>  		if (hci_pi(sk)->channel == HCI_CHANNEL_USER) {
> -			hci_dev_close(hdev->id);
>  			hci_dev_clear_flag(hdev, HCI_USER_CHANNEL);
> +			hci_dev_close(hdev->id);
>  			mgmt_index_added(hdev);
>  		}

Thanks for catching this and coming up with a patch proposal!

My main concern is that there's code within the hci_dev_close() path
that assumes HCI_USER_CHANNEL may be set. E.g. this in
hci_dev_do_close() (which hci_dev_close calls):

	if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
	    !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
	    test_bit(HCI_UP, &hdev->flags)) {
		/* Execute vendor specific shutdown routine */
		if (hdev->shutdown)
			hdev->shutdown(hdev);
	}

With your change the hdev->shutdown() callback would get called which
seems to be what the above if-statement tries to protect against.

Johan