netdev - Re: [PATCH 1/1] bluetooth: Check for SCO type before setting retransmission effort

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <8B07B86E-19D4-49E2-B471-DAF31C8D4F34@holtmann.org>
Date:	Wed, 17 Sep 2014 06:55:26 -0700
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Bernhard Thaler <bernhard.thaler@...t.at>
Cc:	"Gustavo F. Padovan" <gustavo@...ovan.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	BlueZ development <linux-bluetooth@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] bluetooth: Check for SCO type before setting  retransmission effort

Hi Bernhard,

> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort 
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
> 
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
> 
> Signed-off-by: Bernhard Thaler <bernhard.thaler@...t.at>
> ---
> net/bluetooth/hci_conn.c |   39 ++++++++++++++++++++++-----------------
> 1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index faff624..093abcc 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -186,26 +186,31 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> 	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
> 	cp.voice_setting  = cpu_to_le16(conn->setting);
> 
> -	switch (conn->setting & SCO_AIRMODE_MASK) {
> -	case SCO_AIRMODE_TRANSP:
> -		if (conn->attempt > ARRAY_SIZE(sco_param_wideband))
> -			return false;
> -		cp.retrans_effort = 0x02;
> -		param = &sco_param_wideband[conn->attempt - 1];
> -		break;
> -	case SCO_AIRMODE_CVSD:
> -		if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> +	if (!(conn->link->features[0][3] & LMP_ESCO)) {

so we have something that is called lmp_esco_capable.

> +		cp.retrans_effort = 0xff;
> +		cp.pkt_type = __cpu_to_le16(conn->pkt_type);
> +		cp.max_latency = __cpu_to_le16(0xffff);

Anyway, I do not thing this is the right approach. I think what we should do is just reject a connection attempt for transparent eSCO connections (since we only use them for wideband speech anyway) when connecting to a device that has no eSCO support.

> +	} else {
> +		switch (conn->setting & SCO_AIRMODE_MASK) {
> +		case SCO_AIRMODE_TRANSP:
> +			if (conn->attempt > ARRAY_SIZE(sco_param_wideband))
> +				return false;
> +			cp.retrans_effort = 0x02;
> +			param = &sco_param_wideband[conn->attempt - 1];
> +			break;
> +		case SCO_AIRMODE_CVSD:
> +			if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> +				return false;
> +			cp.retrans_effort = 0x01;
> +			param = &sco_param_cvsd[conn->attempt - 1];
> +			break;

For CVSD based attempts, we should start with non eSCO parameter settings if we know that the remote side can not support eSCO.

The reason is that you still need to follow through with defined parameter sets from the HFP profile.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html