greatscottgadgets/luna

USBStreamOutEndpoint accepts packets destined for other devices

zyp opened this issue · 0 comments

zyp commented

Hi, @mubes discovered that sending data to other devices on the same hub interferes with our LUNA-based device. It turns out that the receive filtering is insufficient so that we sometimes receive a packet not destined for us, determine it's an invalid command and return an error response that then upsets the host software.

The filtering in USBStreamOutEndpoint looks like this:

endpoint_number_matches  = (tokenizer.endpoint == self._endpoint_number)
targeting_endpoint       = endpoint_number_matches & tokenizer.is_out

The filtering in USBTokenDetector (tokenizer) looks like this:

m.d.comb += [
    self.interface.is_in     .eq(self.interface.pid == USBPacketID.IN),
    self.interface.is_out    .eq(self.interface.pid == USBPacketID.OUT),
    self.interface.is_setup  .eq(self.interface.pid == USBPacketID.SETUP),
    self.interface.is_ping   .eq(self.interface.pid == USBPacketID.PING)
]

# snip…

token_applicable = (token_data[0:7] == self.address) if self.filter_by_address else True
with m.If(token_applicable):
    m.d.usb += [
        self.interface.pid        .eq(current_pid),
        self.interface.new_token  .eq(1),

        Cat(self.interface.address, self.interface.endpoint).eq(token_data)
    ]

The effect of this is that after receiving an OUT token, tokenizer.is_out and tokenizer.endpoint will retain their state until another endpoint on this device is polled. This means that USBStreamOutEndpoint will receive all DATA packets on the bus in the meantime.

On a typical device that also got IN endpoints that are continously polled, the amount of time it is susceptible to pick up other DATA packets is fairly low, so it's most reliably reproduced by sending the other device enough data to hit flow control, causing the host to continously retry.

Here are some bus captures illustrating the problem. Disregard all the red, these are captured downstream of the hub, so the analyzer only gets the downstream data to the other device, not the upstream data from it, and therefore flags it as incomplete. Our device has address 26 and the other device has address 18.

Here's a normal working case:
image
Our device receives a command and returns a response without issue.

Here's what triggers the issue:
image
Host is trying to send some data to the other device, and one of the attempts falls between our request and response and ends up being queued up as another request. This causes an error response to be queued up and returned instead of the actual response to the next request after this (not visible).

From glancing at the code, I believe the eptri OutFIFOInterface is susceptible to the same issue, but we're not using that so I don't have a convenient way to test.

Expect a PR once I've had time to try implementing and testing a fix.