Re: [PATCH] Bluetooth: Add conn type to identify addr type with SMP over BR/EDR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jiangbo,

On 18 October 2016 at 22:32, Szymon Janc <szymon.janc@xxxxxxxxxxx> wrote:
> Hi Jiangbo,
>
> On Tuesday, 18 October 2016 18:23:38 CEST Wu,Jiangbo wrote:
>> Hi,
>>
>> On Mon, Oct 17, 2016 at 11:05:33PM +0200, Szymon Janc wrote:
>> > Hi,
>> >
>> > On Saturday, 15 October 2016 00:43:13 CEST wujiangbo wrote:
>> > > On Fri, Oct 14, 2016 at 05:19:38PM +0300, Johan Hedberg wrote:
>> > > > Hi Jiangbo,
>> > > >
>> > > > Please don't top-post on this list.
>> > > >
>> > > > On Fri, Oct 14, 2016, Wu, Jiangbo wrote:
>> > > > > If pair a device that unpair firstly that remove encryption key,
>> > > > > encryption key event will be emitted. kernel will receive
>> > > > > 'L2CAP_CID_SMP_BREDR' frame, and then it will use SMP to distribute
>> > > > > key.  SMP would like to use LTK, IRK and CRSK to notify user. If it
>> > > > > don't identify device by which conn type they are, only marks LE as
>> > > > > the device type,
>> > > >
>> > > > Why would that happen? Before SMP over BR/EDR happens pairing would
>> > > > have
>> > > > happened over BR/EDR, so bluetoothd should know that BR/EDR is
>> > > > supported
>> > > > as well (it would even be aware of an existing BR/EDR connection). Are
>> > > > you perhaps trying to work around some bluetoothd bug with all this?
>> > >
>> > > I use upstream bluez source code without change.
>> > >
>> > > Yes, bluetoothd scan will find device type is BR/EDR or LE. As my case,
>> > > device is BR/EDR. But if kernel report CRSK notify, bluetoothd will
>> > > change
>> > >
>> > > the device type to LE. The code you can see:
>> > >   new_csrk_callback -> btd_adapter_get_device ->
> btd_adapter_find_device
>> > >
>> > >           if (bdaddr_type == BDADDR_BREDR)
>> > >
>> > >                   device_set_bredr_support(device);
>> > >
>> > >           else
>> > >
>> > >                   device_set_le_support(device, bdaddr_type);
>> > >
>> > > As Marcel mentioned before, LTK, IRK and CRSK are only valid for LE
>> > > link.
>> > > So the rootcause is why remote start to pair a BR/EDR device, the kernel
>> > > will receive CRSK event.
>> > >
>> > > This is the first pair, and it will pair success even if receive CRSK
>> > > notify. And the second and the next all pair will be failed with remote
>> > > device unpair and then pair again.
>> > >
>> > > > > while Bluetoothd will use this 'addr' and 'addr type' to reply the
>> > > > > comfirm to kernel.
>> > > >
>> > > > What reply are you talking about? There's no user interaction involved
>> > > > with SMP over BR/EDR - that would already have occurred when SSP over
>> > > > BR/EDR happened.
>> > >
>> > > Sorry to confuse the case, the pairing failed coming with next pair
>> > > procedure. Because at the last pair with CRSK notify, device type will
>> > > be
>> > > changed to LE, following is the failed scenario after last success with
>> > > CRSK notify. Remote unpair and pair again.
>> > >
>> > > This reply is SPP, user confirm passkey reply. When pairing proceduce,
>> > > User
>> > > confirm the pairing request through bluetoothd, that will send mgmt op
>> > > 'MGMT_OP_USER_CONFIRM_REPLY' with device address and device type in
>> > > mgmt_cp_user_confirm_reply. Kernel use the device address and type to
>> > > lookup hci conn. Unfortunately, it will lookup hci_conn from LE
>> > > hashtable, that don't include hci conn. So spp reply couldn't send to
>> > > remote, caused pair failed.
>> > >
>> > > > > At the same time kernel always uses them to lookup hci_conn in LE
>> > > > > hashtable firstly, because addr type always marks as LE. Obviously
>> > > > > it
>> > > > > will failed with SMP over BR/EDR.
>> > > >
>> > > > I don't follow this either since there shouldn't have been any "reply"
>> > > > from user space for SMP over BR/EDR. All there should be are events
>> > > > from
>> > > > the kernel for the generated LE keys.
>> > > >
>> > > > > Actually, SPM is only for LE in SPEC,
>> > > >
>> > > > That's not true. SMP is specified both for LE-U and ACL-U.
>> > > >
>> > > > > but kernel already support and use SMP over BR/EDR. if BR/EDR
>> > > > > exchanges key with SMP, it will never reply pairing response to
>> > > > > remote, in other words it will be never paired, that is happened in
>> > > > > our products.
>> > > >
>> > > > Szymon recently implemented SMP over BR/EDR for Zephyr and used
>> > > > Linux/BlueZ as a reference for testing. He didn't report any issues
>> > > > like
>> > > > this. It might help if you could provide some logs (particularly
>> > > > HCI/btmon but also from bluetoothd) to understand what's the actual
>> > > > issue you're seeing.
>> > > >
>> > > > Johan
>> > >
>> > > Sorry to confuse this issue, the log is not in my hand right now,
>> > > so it maybe later.
>> >
>> > So I was able to reproduce this issue. This is bluetoothd bug and not
>> > kernel one. This bug is no specific to cross-transport pairing. It can
>> > happen with any dual-mode device that is doing BR/EDR pairing while being
>> > known as dual mode by bluetoothd when agent replies with passkey or
>> > confirmation.
>> >
>> > To fix this we probably need to hold extra information in
>> > 'struct authentication_req' in device.c about type of pairing (LE or
>> > BR/EDR). This is not a one-liner-fix so I don't have a patch ready yet.
>>
>> Totally agree with you about dual-mode device pairing known as dual mode.
>> But i want to known is that reasonable about device is to do BR/EDR pairing
>> will generate CRSK notify? I'm very intersting about this fixing, this bug
>> is hight priority in our product. In my opinion hold extra informatin in
>> 'struct authentication_req' may not fix this bug. Because if CRSK event is
>> still report, then bluetoothd will change the device type to LE even if we
>> pair device that is scaned with BR/EDR. So i think the rootcase is find
>> does CRSK event make sense in BR/EDR pairing, and how to handle CRSK events
>> in BR/EDR pairing if it make sense. I'm confuse with those.
>
> It doesn't change the device to LE but to dual mode device. This is
> *cross-transport* pairing so keys for other transport are generated.
> baddr_type specify only LE address type, not BR/EDR since there is no address
> type for BR/EDR. This is mostly true but few places in bluetoothd seem to
> asusme that for device supporting BR/EDR type is equal 0. Which is not true if
> device is dual mode.
>
> You should be able to reproduce this bug with dual-mode devices that don't
> support cross-transport pairing: enable advertising, scan from linux, when
> device is found stop advertising and make device discoverable over BR/EDR
> (inquiry). When device is found over BR/EDR stop scanning and start pairing.
>
>>
>> I noticed that if quikly reply the passkey confirm, this bug always be
>> reproduced, but if wait for 2~3s to reply the passkey confirm, it works
>> well every time. In terms of code, wait for 2~3s will cause l2cap chan
>> timeout for info timer that created by HCI_EV_REMOTE_EXT_FEATURES event,
>> and timeout will change l2cap chan to BT_CONNECTED. So next SMP
>> resume/ready don't distribute key also CRSK events.
>>
>> It can't reproduce with btmgmt, because it reply passkey confirm always only
>> use BR/EDR in 'struct mgmt_cp_user_confirm_reply' not use device relation
>> type.
>>
>> bluetoothd.log and btmon.log are attached. It records two pair request
>> sequence, one is pair success that have CRSK event, another is next pair
>> reqeust don't success any, hope those maybe help you to annlyze this bug.

I've sent a patch "[RFC] core: Fix BR/EDR pairing for dual mode devices".
Please check if this solves issue you are seeing.

-- 
pozdrawiam
Szymon K. Janc
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux