Skip to content

Commit

Permalink
Merge pull request #192 from martinling/iso-fix
Browse files Browse the repository at this point in the history 
Fix interpretation of isochronous transactions, including ambiguous cases
  • Loading branch information
@miek
miek authored Sep 21, 2024
2 parents d0a36b3 + d2308a6 commit 282b54e
Show file tree
Hide file tree
Showing 8 changed files with 1,766 additions and 34 deletions.
106 changes: 85 additions & 21 deletions src/capture.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::util::{fmt_count, fmt_size};
use anyhow::{Context, Error, bail};
use arc_swap::{ArcSwap, ArcSwapOption};
use bytemuck_derive::{Pod, Zeroable};
use itertools::Itertools;
use num_enum::{IntoPrimitive, FromPrimitive};

// Use 2MB block size for packet data, which is a large page size on x86_64.
Expand Down Expand Up @@ -303,7 +304,7 @@ pub const FRAMING_EP_NUM: EndpointNum = EndpointNum(0x11);
pub const INVALID_EP_ID: EndpointId = EndpointId::constant(0);
pub const FRAMING_EP_ID: EndpointId = EndpointId::constant(1);

#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum EndpointType {
Unidentified,
Framing,
Expand All @@ -327,6 +328,7 @@ pub struct DeviceData {
pub device_descriptor: ArcSwapOption<DeviceDescriptor>,
pub configurations: ArcSwap<VecMap<ConfigNum, Arc<Configuration>>>,
pub config_number: ArcSwapOption<ConfigNum>,
pub interface_settings: ArcSwap<VecMap<InterfaceNum, InterfaceAlt>>,
pub endpoint_details: ArcSwap<VecMap<EndpointAddr, EndpointDetails>>,
pub strings: ArcSwap<VecMap<StringId, UTF16ByteVec>>,
pub version: AtomicU32,
Expand Down Expand Up @@ -384,16 +386,19 @@ impl DeviceData {
pub fn update_endpoint_details(&self) {
if let Some(number) = self.config_number.load().as_ref() {
if let Some(config) = &self.configurations.load().get(**number) {
let iface_settings = self.interface_settings.load();
self.endpoint_details.update(|endpoint_details| {
for iface in config.interfaces.values() {
for ep_desc in &iface.endpoint_descriptors {
let ep_addr = ep_desc.endpoint_address;
let ep_type = ep_desc.attributes.endpoint_type();
let ep_max = ep_desc.max_packet_size as usize;
endpoint_details.set(
ep_addr,
(ep_type, Some(ep_max))
);
for ((num, alt), iface) in config.interfaces.iter() {
if iface_settings.get(*num) == Some(alt) {
for ep_desc in &iface.endpoint_descriptors {
let ep_addr = ep_desc.endpoint_address;
let ep_type = ep_desc.attributes.endpoint_type();
let ep_max = ep_desc.max_packet_size as usize;
endpoint_details.set(
ep_addr,
(ep_type, Some(ep_max))
);
}
}
}
});
Expand Down Expand Up @@ -425,6 +430,8 @@ impl DeviceData {
=> self.decode_descriptor_read(fields, payload)?,
(RequestType::Standard, StandardRequest::SetConfiguration)
=> self.decode_configuration_set(fields)?,
(RequestType::Standard, StandardRequest::SetInterface)
=> self.decode_interface_set(fields)?,
_ => ()
}
Ok(())
Expand Down Expand Up @@ -482,6 +489,30 @@ impl DeviceData {
{
let config_number = ConfigNum(fields.value.try_into()?);
self.config_number.swap(Some(Arc::new(config_number)));
let mut interface_settings = VecMap::new();
if let Some(config) = self.configurations.load().get(config_number) {
// All interfaces are reset to setting zero.
for (num, _alt) in config.interfaces
.keys()
.unique_by(|(num, _alt)| num)
{
interface_settings.set(*num, InterfaceAlt(0));
}
}
self.interface_settings.swap(Arc::new(interface_settings));
self.update_endpoint_details();
self.increment_version();
Ok(())
}

fn decode_interface_set(&self, fields: &SetupFields)
-> Result<(), Error>
{
let iface_num = InterfaceNum(fields.index.try_into()?);
let iface_alt = InterfaceAlt(fields.value.try_into()?);
self.interface_settings.update(|interface_settings|
interface_settings.set(iface_num, iface_alt)
);
self.update_endpoint_details();
self.increment_version();
Ok(())
Expand Down Expand Up @@ -528,6 +559,13 @@ pub struct Transaction {
payload_byte_range: Option<Range<Id<u8>>>,
}

#[derive(PartialEq)]
pub enum TransactionResult {
Success,
Failure,
Ambiguous
}

impl Transaction {
fn packet_count(&self) -> u64 {
self.packet_id_range.len()
Expand All @@ -537,17 +575,30 @@ impl Transaction {
self.payload_byte_range.as_ref().map(|range| range.len())
}

fn successful(&self) -> bool {
fn result(&self, ep_type: EndpointType) -> TransactionResult {
use PID::*;
use EndpointType::*;
use usb::EndpointType::*;
use TransactionResult::*;
match (self.start_pid, self.end_pid) {

// SPLIT is successful if it ends with DATA0/DATA1/ACK/NYET.
(SPLIT, DATA0 | DATA1 | ACK | NYET) => true,
(SPLIT, DATA0 | DATA1 | ACK | NYET) => Success,

// SETUP/IN/OUT is successful if it ends with ACK/NYET.
(SETUP | IN | OUT, ACK | NYET) => true,
(SETUP | IN | OUT, ACK | NYET) => Success,

// IN/OUT followed by DATA0/DATA1 depends on endpoint type.
(IN | OUT, DATA0 | DATA1) => match ep_type {
// For an isochronous endpoint this is a success.
Normal(Isochronous) => Success,
// For an unidentified endpoint this is ambiguous.
Unidentified => Ambiguous,
// For any other endpoint type this is a failure (no handshake).
_ => Failure,
},

(..) => false
(..) => Failure
}
}

Expand All @@ -556,6 +607,9 @@ impl Transaction {
use StartComplete::*;
use Direction::*;
use PID::*;
use EndpointType::*;
use usb::EndpointType::*;
use TransactionResult::*;
let end_pid = match (direction, self.start_pid, self.split.as_ref()) {
(In, OUT, None) |
(Out, IN, None) =>
Expand All @@ -572,7 +626,7 @@ impl Transaction {
};
if end_pid == STALL {
Stalled
} else if self.successful() {
} else if self.result(Normal(Control)) == Success {
Completed
} else {
Incomplete
Expand Down Expand Up @@ -1352,6 +1406,7 @@ impl ItemSource<TrafficItem> for CaptureReader {
Transfer(transfer_id) => {
use EndpointType::*;
use usb::EndpointType::*;
use TransactionResult::*;
let entry = self.transfer_index.get(*transfer_id)?;
let endpoint_id = entry.endpoint_id();
let endpoint = self.endpoints.get(endpoint_id)?;
Expand Down Expand Up @@ -1422,8 +1477,8 @@ impl ItemSource<TrafficItem> for CaptureReader {
} else {
count
};
match (first_transaction.successful(), starting) {
(true, true) => {
match (first_transaction.result(ep_type), starting) {
(Success, true) => {
let ep_traf =
self.endpoint_traffic(endpoint_id)?;
let data_range =
Expand All @@ -1448,12 +1503,22 @@ impl ItemSource<TrafficItem> for CaptureReader {
write!(s, ": {display_bytes}")
}
},
(true, false) => write!(s,
(Success, false) => write!(s,
"End of {ep_type_lower} transfer on endpoint {endpoint}"),
(false, true) => write!(s,
(Failure, true) => write!(s,
"Polling {count} times for {ep_type_lower} transfer on endpoint {endpoint}"),
(false, false) => write!(s,
(Failure, false) => write!(s,
"End polling for {ep_type_lower} transfer on endpoint {endpoint}"),
(Ambiguous, true) => {
write!(s, "{count} ambiguous transactions on endpoint {endpoint}")?;
if detail {
write!(s, "\nThe result of these transactions is ambiguous because the endpoint type is not known.")?;
write!(s, "\nTry starting the capture before this device is enumerated, so that its descriptors are captured.")?;
}
Ok(())
},
(Ambiguous, false) => write!(s,
"End of ambiguous transactions."),
}
}
}?;
Expand Down Expand Up @@ -1819,7 +1884,6 @@ mod tests {
use std::path::PathBuf;
use crate::decoder::Decoder;
use crate::pcap::Loader;
use itertools::Itertools;

fn summarize_item(cap: &mut CaptureReader, item: &TrafficItem, depth: usize)
-> String
Expand Down
45 changes: 36 additions & 9 deletions src/decoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ enum TransactionStatus {
Retry,
Done,
Fail,
Ambiguous,
Invalid
}

Expand Down Expand Up @@ -80,6 +81,7 @@ struct TransactionState {
id: TransactionId,
last: PID,
endpoint_id: Option<EndpointId>,
endpoint_type: Option<usb::EndpointType>,
ep_transaction_id: Option<EndpointTransactionId>,
setup: Option<SetupFields>,
payload: Option<Vec<u8>>,
Expand Down Expand Up @@ -110,7 +112,9 @@ fn transaction_status(state: &Option<TransactionState>, packet: &[u8])
},
Some(TransactionState {
style: Simple(first),
last, ..}) =>
last,
endpoint_type,
..}) =>
{
match (first, last, next) {
// These tokens always start a new transaction.
Expand All @@ -132,7 +136,15 @@ fn transaction_status(state: &Option<TransactionState>, packet: &[u8])
// IN may be followed by NAK or STALL, failing transaction.
(_, IN, NAK | STALL) => Fail,
// IN or OUT may be followed by DATA0 or DATA1.
(_, IN | OUT, DATA0 | DATA1) if packet.len() >= 3 => Continue,
(_, IN | OUT, DATA0 | DATA1) =>
match endpoint_type {
// No handshake for an isochronous transaction.
Some(Isochronous) => Done,
// Expect handshake if known to be non-isochronous.
Some(_) => Continue,
// If we don't know the endpoint type, we can't be sure.
None => Ambiguous,
},
// An ACK or NYET then completes the transaction.
(IN | OUT, DATA0 | DATA1, ACK | NYET) => Done,
// OUT may also be completed by NAK or STALL.
Expand Down Expand Up @@ -394,7 +406,7 @@ impl EndpointData {
}
if complete {
self.last_success = success;
if success && short {
if success && short && ep_type != Normal(Isochronous) {
// New transfer, ended immediately by a short packet.
Single
} else {
Expand Down Expand Up @@ -423,7 +435,7 @@ impl EndpointData {
let success_changed = success != self.last_success;
self.last_success = success;
if success_changed {
if success && short {
if success && short && ep_type != Normal(Isochronous) {
// New transfer, ended immediately by a short packet.
Single
} else {
Expand All @@ -432,7 +444,7 @@ impl EndpointData {
}
} else if success {
// Continuing an ongoing transfer.
if short {
if short && ep_type != Normal(Isochronous) {
// A short packet ends the transfer.
Done
} else {
Expand Down Expand Up @@ -666,6 +678,9 @@ impl Decoder {
self.transaction_append(pid, packet)?;
self.transaction_end(success, complete)?;
},
Ambiguous => {
self.transaction_append(pid, packet)?;
},
Invalid => {
self.transaction_start(packet_id, pid, packet)?;
self.transaction_end(false, false)?;
Expand All @@ -683,19 +698,31 @@ impl Decoder {
use PID::*;
use TransactionStyle::*;
let transaction_id = self.capture.transaction_index.push(packet_id)?;
let (style, endpoint_id) = match pid {
Malformed => (Simple(pid), Some(INVALID_EP_ID)),
let (style, endpoint_id, endpoint_type) = match pid {
Malformed => (Simple(pid), Some(INVALID_EP_ID), None),
SPLIT => {
let split = SplitFields::from_packet(packet);
(Split(split.sc(), split.endpoint_type(), None), None)
let style = Split(split.sc(), split.endpoint_type(), None);
(style, None, Some(split.endpoint_type()))
},
pid => (Simple(pid), Some(self.packet_endpoint(pid, packet)?))
pid => {
let endpoint_id = self.packet_endpoint(pid, packet)?;
let ep_data = &self.endpoint_data[endpoint_id];
let dev_data = self.capture.device_data(ep_data.device_id)?;
let (ep_type, _) = dev_data.endpoint_details(ep_data.address);
let endpoint_type = match ep_type {
EndpointType::Normal(usb_ep_type) => Some(usb_ep_type),
_ => None,
};
(Simple(pid), Some(endpoint_id), endpoint_type)
}
};
let mut state = TransactionState {
style,
id: transaction_id,
last: pid,
endpoint_id,
endpoint_type,
ep_transaction_id: None,
setup: None,
payload: None,
Expand Down
7 changes: 3 additions & 4 deletions src/usb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub fn validate_packet(packet: &[u8]) -> Result<PID, Option<PID>> {
macro_rules! byte_type {
($name: ident) => {
#[derive(Copy, Clone, Debug, Default,
PartialEq, Eq, PartialOrd, Ord,
PartialEq, Eq, Hash, PartialOrd, Ord,
Pod, Zeroable, From, Into, Display)]
#[repr(transparent)]
pub struct $name(pub u8);
Expand Down Expand Up @@ -428,9 +428,8 @@ impl StandardRequest {
},
GetConfiguration => format!("Getting configuration"),
SetConfiguration => format!("Setting configuration {}", fields.value),
GetInterface => format!("Getting interface {}", fields.index),
SetInterface => format!("Setting interface {} to {}",
fields.index, fields.value),
GetInterface => format!("Getting interface setting"),
SetInterface => format!("Setting alternate setting {}", fields.value),
SynchFrame => format!("Synchronising frame"),
Unknown => format!("Unknown standard request"),
}
Expand Down
Binary file added tests/iso-ambiguous/capture.pcap
View file
Binary file not shown.
14 changes: 14 additions & 0 deletions tests/iso-ambiguous/reference.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
18 ambiguous transactions on endpoint 27.3 IN
IN transaction on 27.3 with 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
IN packet on 27.3, CRC 0B
DATA0 packet with CRC E17B and 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
IN transaction on 27.3 with 64 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
IN packet on 27.3, CRC 0B
DATA0 packet with CRC D0BF and 64 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
16 times: IN transaction on 27.3 with 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
IN packet on 27.3, CRC 0B
DATA0 packet with CRC E17B and 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
7 ambiguous transactions on endpoint 27.3 OUT
7 times: OUT transaction on 27.3 with 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
OUT packet on 27.3, CRC 0B
DATA0 packet with CRC E17B and 192 data bytes: [00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]...
Binary file added tests/iso-unambiguous/capture.pcap
View file
Binary file not shown.
Loading

0 comments on commit 282b54e

Please sign in to comment.