[Bluez-devel] Questions about correctness of hci_usb sco support.

[Bluez-devel] Questions about correctness of hci_usb sco support.

[James Courtier-Dutton]

In hci_usb.c file, line 604 ish
static inline int __recv_frame(struct hci_usb *husb, int type, void 
*data, int count)

Contains: -
case HCI_SCODATA_PKT:
        if (count >= HCI_SCO_HDR_SIZE) {
                  struct hci_sco_hdr *h = data;
                  len = HCI_SCO_HDR_SIZE + h->dlen;
        } else
                  return -EILSEQ;
        break;

With a SCO HCI packet, it lasts 3 air frames for a bluetooth headset.
How do we know that the first frame we receive from the usb bluetooth 
device is the SCO HCI header?
What happens if the first SCO HCI frame we receive is actually the 
second or third frame in the SCO HCI packet ?
Surely some validation checks need to be done.
For example, depending on the sample format we are using, we should 
already know what the SCO HCI length should be, so we could check this 
against the length in the SCO HCI header, and only accept the frame if 
they match, if they don't match, drop the frame, and wait for the next 
frame.
I would expect similar problems with HCI int/bulk frames, but I don't 
actually see any corrupt int/bulk frames, so I was wondering whether the 
usb bluetooth dongle somehow ensures that the first air frame we receive 
is actually the start of an HCI frame. Maybe it is just luck, as 
int/bulk frames normally have a lot of blank invalid frames in between, 
so maybe as soon as it sees a valid frame, it is always the start of the 
int/bulk frame. I don't think we can make this assumtion all the time, 
in case we start filling the air entirely with bulk frames, and some air 
frames get lost. We will have to drop the hci frame, and then resync 
when the next hci frame arrives.

With SCO HCI frames, there are never gaps between frames, so if we loose 
a single air frame, we would have to somehow resync to get back to the 
SCO HCI header frame.

Can anyone help me understand this?

Cheers
James


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Questions about correctness of hci_usb sco support.

[Marcel Holtmann]

Hi James,

> In hci_usb.c file, line 604 ish
> static inline int __recv_frame(struct hci_usb *husb, int type, void 
> *data, int count)
> 
> Contains: -
> case HCI_SCODATA_PKT:
>         if (count >= HCI_SCO_HDR_SIZE) {
>                   struct hci_sco_hdr *h = data;
>                   len = HCI_SCO_HDR_SIZE + h->dlen;
>         } else
>                   return -EILSEQ;
>         break;
> 
> With a SCO HCI packet, it lasts 3 air frames for a bluetooth headset.
> How do we know that the first frame we receive from the usb bluetooth 
> device is the SCO HCI header?

	if (!skb) {
		/* Start of the frame */

> What happens if the first SCO HCI frame we receive is actually the 
> second or third frame in the SCO HCI packet ?
> Surely some validation checks need to be done.
> For example, depending on the sample format we are using, we should 
> already know what the SCO HCI length should be, so we could check this 
> against the length in the SCO HCI header, and only accept the frame if 
> they match, if they don't match, drop the frame, and wait for the next 
> frame.
> I would expect similar problems with HCI int/bulk frames, but I don't 
> actually see any corrupt int/bulk frames, so I was wondering whether the 
> usb bluetooth dongle somehow ensures that the first air frame we receive 
> is actually the start of an HCI frame. Maybe it is just luck, as 
> int/bulk frames normally have a lot of blank invalid frames in between, 
> so maybe as soon as it sees a valid frame, it is always the start of the 
> int/bulk frame. I don't think we can make this assumtion all the time, 
> in case we start filling the air entirely with bulk frames, and some air 
> frames get lost. We will have to drop the hci frame, and then resync 
> when the next hci frame arrives.

I actually don't get your point, because the USB INT, BULK and ISOC
URB's has nothing to do with the frames on the air. It is the HCI of the
Bluetooth chip.

Regards

Marcel




-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Questions about correctness of hci_usb sco support.

[James Courtier-Dutton]

Marcel Holtmann wrote:
> Hi James,
> 
> 
>>In hci_usb.c file, line 604 ish
>>static inline int __recv_frame(struct hci_usb *husb, int type, void 
>>*data, int count)
>>
>>Contains: -
>>case HCI_SCODATA_PKT:
>>        if (count >= HCI_SCO_HDR_SIZE) {
>>                  struct hci_sco_hdr *h = data;
>>                  len = HCI_SCO_HDR_SIZE + h->dlen;
>>        } else
>>                  return -EILSEQ;
>>        break;
>>
>>With a SCO HCI packet, it lasts 3 air frames for a bluetooth headset.
>>How do we know that the first frame we receive from the usb bluetooth 
>>device is the SCO HCI header?
> 
> 
> 	if (!skb) {
> 		/* Start of the frame */
> 
> 
>>What happens if the first SCO HCI frame we receive is actually the 
>>second or third frame in the SCO HCI packet ?
>>Surely some validation checks need to be done.
>>For example, depending on the sample format we are using, we should 
>>already know what the SCO HCI length should be, so we could check this 
>>against the length in the SCO HCI header, and only accept the frame if 
>>they match, if they don't match, drop the frame, and wait for the next 
>>frame.
>>I would expect similar problems with HCI int/bulk frames, but I don't 
>>actually see any corrupt int/bulk frames, so I was wondering whether the 
>>usb bluetooth dongle somehow ensures that the first air frame we receive 
>>is actually the start of an HCI frame. Maybe it is just luck, as 
>>int/bulk frames normally have a lot of blank invalid frames in between, 
>>so maybe as soon as it sees a valid frame, it is always the start of the 
>>int/bulk frame. I don't think we can make this assumtion all the time, 
>>in case we start filling the air entirely with bulk frames, and some air 
>>frames get lost. We will have to drop the hci frame, and then resync 
>>when the next hci frame arrives.
> 
> 
> I actually don't get your point, because the USB INT, BULK and ISOC
> URB's has nothing to do with the frames on the air. It is the HCI of the
> Bluetooth chip.
> 
> Regards
> 
> Marcel
> 
> 
> 
> 

__recv_frame()  receives a frame from the USB interface.
It then joins up frames to create a full HCI packet to send to higher 
layers.
"struct sk_buff *skb = __reassembly(husb, type);"

So we have a skb for each HCI type.
The skb will not exist the first time we receive an frame of a 
particular type.
The current code always assumes that the first frame it receives of a 
particular type will always be the first frame of an HCI packet that 
might consist of multiple frames.
I can't understand how we can be 100% that the first frame seen is 
always the first frame of the HCI packet.
I can't see why we cannot ever see a situation where the first frame 
received of a particular type might instead be the second frame of the 
HCI packet. As the __recv_frame() uses the contents of that first frame 
to control the reassembly process. How can we be sure that that first 
frame does in fact contain the first frame of a valid HCI packet?
E.g. If a remote bluetooth device somehow creates an HCI packet with 
bogus HCI header, surely this could (worst case) crash the kernel?

E.g.

Frames coming from USB.
1) HCI header+data (Header contains details of how many frames are in 
this HCI packet via a packet length field, e.g 27 bytes, or 3 frames)
2) HCI data
3) HCI data

What happens if there is an error in the HCI header bytes, or frame (1) 
is somehow lost, so it then thinks frame (2) contains the HCI header+data ?

Summary: -
The current code works well if everything is very well behaved, but what 
happens if errors occur, or is there some mechanism to prevent any 
errors that I am not currently away of?

Cheers
James

Re: [Bluez-devel] Questions about correctness of hci_usb sco support.

[Marcel Holtmann]

Hi James,

> __recv_frame()  receives a frame from the USB interface.
> It then joins up frames to create a full HCI packet to send to higher 
> layers.
> "struct sk_buff *skb = __reassembly(husb, type);"
> 
> So we have a skb for each HCI type.
> The skb will not exist the first time we receive an frame of a 
> particular type.
> The current code always assumes that the first frame it receives of a 
> particular type will always be the first frame of an HCI packet that 
> might consist of multiple frames.
> I can't understand how we can be 100% that the first frame seen is 
> always the first frame of the HCI packet.
> I can't see why we cannot ever see a situation where the first frame 
> received of a particular type might instead be the second frame of the 
> HCI packet. As the __recv_frame() uses the contents of that first frame 
> to control the reassembly process. How can we be sure that that first 
> frame does in fact contain the first frame of a valid HCI packet?
> E.g. If a remote bluetooth device somehow creates an HCI packet with 
> bogus HCI header, surely this could (worst case) crash the kernel?

I don't really see a problem here, because remote devices has nothing do
to with the local HCI. We can only be in trouble if we lost an URB.

Regards

Marcel




-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel