Hi Marcel, On Fri, Jul 17, 2020 at 2:51 AM Marcel Holtmann <mar...@holtmann.org> wrote: > > Hi Alain, > > > >>> There is a possibility that an ACL packet is received before we > > >>> receive the HCI connect event for the corresponding handle. If this > > >>> happens, we discard the ACL packet. > > >>> > > >>> Rather than just ignoring them, this patch provides a queue for > > >>> incoming ACL packet without a handle. The queue is processed when > > >>> receiving a HCI connection event. If 2 seconds elapsed without > > >>> receiving the HCI connection event, assume something bad happened > > >>> and discard the queued packet. > > >>> > > >>> Signed-off-by: Archie Pusaka <apus...@chromium.org> > > >>> Reviewed-by: Abhishek Pandit-Subedi <abhishekpan...@chromium.org> > > >> > > >> so two things up front. I want to hide this behind a > > >> HCI_QUIRK_OUT_OF_ORDER_ACL that a transport driver has to set first. > > >> Frankly if this kind of out-of-order happens on UART or SDIO transports, > > >> then something is obviously going wrong. I have no plan to fix up after > > >> a fully serialized transport. > > >> > > >> Secondly, if a transport sets HCI_QUIRK_OUT_OF_ORDER_ACL, then I want > > >> this off by default. You can enable it via an experimental setting. The > > >> reason here is that we have to make it really hard and fail as often as > > >> possible so that hardware manufactures and spec writers realize that > > >> something is fundamentally broken here. > > I don't have any objection to making this explicit enable to non serialized > > transports. However, I do wonder what the intention is around making this > > off by default. We already know there is a race condition between the > > interupt and bulk endpoints over USB, so this can and does happen. > > Hardware manufaturers can't relly do much about this other than trying to > > pull the interupt endpoint more often, but that's only a workaround, it > > can't avoid it all together. > > > > IMO, this seems like a legitimate fix at the host level and I don't see any > > obvious benefits to hide this fix under an experimental feature and make it > > more difficult for the customers and system integrators to discover. > > the problem is that this is not a fix. It is papering over a hole and at best > a workaround with both eyes closed and hoping for the best. I am not looking > forward for the first security researcher to figure out that they have a > chance to inject an unencrypted packet since we are waiting 2 seconds for the > USB transport to get its act together. I don't think this is the right characterization but I agree, 2 seconds would be too long, it would ideally be no longer than the USB polling interval diff.
> > In addition, I think that Luiz attempt to align with the poll intervals > inside the USB transport directly is a cleaner and more self-contained > approach. It also reduces the window of opportunity for any attacker since we > actually align the USB transport specific intervals with each other. I'll have to look at Luiz's patch and think through if this really eliminates the problem. If may indeed be a more practical approach to this problem. > > Regards > > Marcel >
