From 6529d00348245cafe49451bff7ba5d7b3f804363 Mon Sep 17 00:00:00 2001
From: Yun-Hao Chung <howardchung@google.com>
Date: Mon, 19 Feb 2024 08:31:10 +0000
Subject: [PATCH] Floss: Hcidoc: Add external controller signal
Some users plug an external controller, if the stack somehow use that,
it is likely to fail in various situation as it wasn't tested.
This adds a signal to detect if the chosen controller is an external
controller by checking if the local name and manufacture are in the
known list and also tracking the new index event.
Bug: 325403872
Test: m -j
Test: Using report 91563648247, verify every read local name and the new
index are reported.
Tag: #floss
Flag: EXEMPT, Floss only change
Change-Id: Ia25c4f8deaaa20d4529d412938c3cc1fdcf915c0
---
floss/hcidoc/Cargo.toml | 1 +
floss/hcidoc/src/groups/connections.rs | 3 +
floss/hcidoc/src/groups/controllers.rs | 95 +++++++++++++++++++++++-
floss/hcidoc/src/groups/informational.rs | 5 +-
floss/hcidoc/src/parser.rs | 47 ++++++++++++
5 files changed, 146 insertions(+), 5 deletions(-)
diff --git a/floss/hcidoc/Cargo.toml b/floss/hcidoc/Cargo.toml
index ce6437943b2..d7f279760d0 100644
--- a/floss/hcidoc/Cargo.toml
+++ b/floss/hcidoc/Cargo.toml
@@ -10,3 +10,4 @@ clap = "4.0"
chrono = "0.4"
num-derive = "0.3"
num-traits = "0.2"
+lazy_static = "1.0"
diff --git a/floss/hcidoc/src/groups/connections.rs b/floss/hcidoc/src/groups/connections.rs
index 509eb7f7388..765f7eed928 100644
--- a/floss/hcidoc/src/groups/connections.rs
+++ b/floss/hcidoc/src/groups/connections.rs
@@ -801,6 +801,9 @@ impl Rule for OddDisconnectionsRule {
// We don't do anything with RX packets yet.
PacketChild::AclRx(_) => (),
+
+ // End packet.inner match
+ _ => (),
}
}
diff --git a/floss/hcidoc/src/groups/controllers.rs b/floss/hcidoc/src/groups/controllers.rs
index 3682814796f..46927607658 100644
--- a/floss/hcidoc/src/groups/controllers.rs
+++ b/floss/hcidoc/src/groups/controllers.rs
@@ -1,35 +1,60 @@
///! Rule group for tracking controller related issues.
use chrono::NaiveDateTime;
+use lazy_static::lazy_static;
+use std::collections::HashSet;
use std::convert::Into;
use std::io::Write;
use crate::engine::{Rule, RuleGroup, Signal};
-use crate::parser::{Packet, PacketChild};
-use bt_packets::hci::EventChild;
+use crate::parser::{NewIndex, Packet, PacketChild};
+use bt_packets::hci::{CommandCompleteChild, ErrorCode, EventChild, LocalVersionInformation};
enum ControllerSignal {
- HardwareError, // Controller reports HCI event: Hardware Error
+ HardwareError, // Controller reports HCI event: Hardware Error
+ LikelyExternalController, // Controller is not in the known list. Likely to be an external controller.
}
impl Into<&'static str> for ControllerSignal {
fn into(self) -> &'static str {
match self {
ControllerSignal::HardwareError => "HardwareError",
+ ControllerSignal::LikelyExternalController => "LikelyExternalController",
}
}
}
+lazy_static! {
+ static ref KNOWN_CONTROLLER_NAMES: [String; 6] = [
+ String::from("Bluemoon Universal Bluetooth Host Controller"), // AC7625
+ String::from("MTK MT7961 #1"), // MT7921LE/MT7921LS
+ String::from("MTK MT7922 #1"), // MT7922
+ String::from("RTK_BT_5.0"), // RTL8822CE
+ String::from("RT_BT"), // RTL8852AE
+ String::from(""), // AC9260/AC9560/AX200/AX201/AX203/AX211/MVL8897/QCA6174A3/QCA6174A5/QC_WCN6856
+ ];
+}
+const KNOWN_CONTROLLER_MANUFACTURERS: [u16; 5] = [
+ 2, // Intel.
+ 29, // Qualcomm
+ 70, // MediaTek
+ 72, // Marvell
+ 93, // Realtek
+];
+
struct ControllerRule {
/// Pre-defined signals discovered in the logs.
signals: Vec<Signal>,
/// Interesting occurrences surfaced by this rule.
reportable: Vec<(NaiveDateTime, String)>,
+
+ /// All detected open_index.
+ controllers: HashSet<String>,
}
impl ControllerRule {
pub fn new() -> Self {
- ControllerRule { signals: vec![], reportable: vec![] }
+ ControllerRule { signals: vec![], reportable: vec![], controllers: HashSet::new() }
}
pub fn report_hardware_error(&mut self, packet: &Packet) {
@@ -41,6 +66,48 @@ impl ControllerRule {
self.reportable.push((packet.ts, format!("controller reported hardware error")));
}
+
+ fn process_local_name(&mut self, local_name: &[u8; 248], packet: &Packet) {
+ let null_index = local_name.iter().position(|&b| b == 0).unwrap_or(local_name.len());
+ match String::from_utf8(local_name[..null_index].to_vec()) {
+ Ok(name) => {
+ if !KNOWN_CONTROLLER_NAMES.contains(&name) {
+ self.signals.push(Signal {
+ index: packet.index,
+ ts: packet.ts,
+ tag: ControllerSignal::LikelyExternalController.into(),
+ })
+ }
+ }
+ Err(_) => self.signals.push(Signal {
+ index: packet.index,
+ ts: packet.ts,
+ tag: ControllerSignal::LikelyExternalController.into(),
+ }),
+ }
+ }
+
+ fn process_local_version(&mut self, version_info: &LocalVersionInformation, packet: &Packet) {
+ if !KNOWN_CONTROLLER_MANUFACTURERS.contains(&version_info.manufacturer_name) {
+ self.signals.push(Signal {
+ index: packet.index,
+ ts: packet.ts,
+ tag: ControllerSignal::LikelyExternalController.into(),
+ })
+ }
+ }
+
+ fn process_new_index(&mut self, new_index: &NewIndex, packet: &Packet) {
+ self.controllers.insert(new_index.get_addr_str());
+
+ if self.controllers.len() > 1 {
+ self.signals.push(Signal {
+ index: packet.index,
+ ts: packet.ts,
+ tag: ControllerSignal::LikelyExternalController.into(),
+ });
+ }
+ }
}
impl Rule for ControllerRule {
@@ -50,8 +117,28 @@ impl Rule for ControllerRule {
EventChild::HardwareError(_ev) => {
self.report_hardware_error(&packet);
}
+ EventChild::CommandComplete(ev) => match ev.specialize() {
+ CommandCompleteChild::ReadLocalNameComplete(ev) => {
+ if ev.get_status() != ErrorCode::Success {
+ return;
+ }
+
+ self.process_local_name(ev.get_local_name(), &packet);
+ }
+ CommandCompleteChild::ReadLocalVersionInformationComplete(ev) => {
+ if ev.get_status() != ErrorCode::Success {
+ return;
+ }
+
+ self.process_local_version(ev.get_local_version_information(), &packet);
+ }
+ _ => {}
+ },
_ => {}
},
+ PacketChild::NewIndex(ni) => {
+ self.process_new_index(ni, &packet);
+ }
_ => {}
}
}
diff --git a/floss/hcidoc/src/groups/informational.rs b/floss/hcidoc/src/groups/informational.rs
index 9ae82c9f191..31181ebf1c6 100644
--- a/floss/hcidoc/src/groups/informational.rs
+++ b/floss/hcidoc/src/groups/informational.rs
@@ -930,7 +930,10 @@ impl Rule for InformationalRule {
// PacketChild::AclRx(rx).specialize()
_ => {}
}
- } // packet.inner
+ }
+
+ // End packet.inner match
+ _ => (),
}
}
diff --git a/floss/hcidoc/src/parser.rs b/floss/hcidoc/src/parser.rs
index bec8df41c17..7d11b983189 100644
--- a/floss/hcidoc/src/parser.rs
+++ b/floss/hcidoc/src/parser.rs
@@ -287,6 +287,7 @@ pub enum PacketChild {
HciEvent(Event),
AclTx(Acl),
AclRx(Acl),
+ NewIndex(NewIndex),
}
impl<'a> TryFrom<&'a LinuxSnoopPacket> for PacketChild {
@@ -314,6 +315,11 @@ impl<'a> TryFrom<&'a LinuxSnoopPacket> for PacketChild {
Err(e) => Err(format!("Couldn't parse acl rx: {:?}", e)),
},
+ LinuxSnoopOpcodes::NewIndex => match NewIndex::parse(item.data.as_slice()) {
+ Ok(data) => Ok(PacketChild::NewIndex(data)),
+ Err(e) => Err(format!("Couldn't parse new index: {:?}", e)),
+ },
+
// TODO(b/262928525) - Add packet handlers for more packet types.
_ => Err(format!("Unhandled packet opcode: {:?}", item.opcode())),
}
@@ -400,3 +406,44 @@ pub fn get_acl_content(acl: &Acl) -> AclContent {
_ => AclContent::None,
}
}
+
+#[derive(Clone, Debug)]
+pub struct NewIndex {
+ _hci_type: u8,
+ _bus: u8,
+ bdaddr: [u8; 6],
+ _name: [u8; 8],
+}
+
+impl NewIndex {
+ fn parse(data: &[u8]) -> Result<NewIndex, std::string::String> {
+ if data.len() != std::mem::size_of::<NewIndex>() {
+ return Err(format!("Invalid size for New Index packet: {}", data.len()));
+ }
+
+ let rest = data;
+ let (hci_type, rest) = rest.split_at(std::mem::size_of::<u8>());
+ let (bus, rest) = rest.split_at(std::mem::size_of::<u8>());
+ let (bdaddr, rest) = rest.split_at(6 * std::mem::size_of::<u8>());
+ let (name, _rest) = rest.split_at(8 * std::mem::size_of::<u8>());
+
+ Ok(NewIndex {
+ _hci_type: hci_type[0],
+ _bus: bus[0],
+ bdaddr: bdaddr.try_into().unwrap(),
+ _name: name.try_into().unwrap(),
+ })
+ }
+
+ pub fn get_addr_str(&self) -> String {
+ String::from(format!(
+ "[{:02X}:{:02X}:{:02X}:{:02X}:{:02X}:{:02X}]",
+ self.bdaddr[0],
+ self.bdaddr[1],
+ self.bdaddr[2],
+ self.bdaddr[3],
+ self.bdaddr[4],
+ self.bdaddr[5]
+ ))
+ }
+}
--
GitLab