From 284530334207809446ed70a5412cd02f426f5aa9 Mon Sep 17 00:00:00 2001 From: Henri Chataing Date: Wed, 19 Apr 2023 18:14:03 +0000 Subject: [PATCH] Fix HCI parser and packetizer Issue found from the test H4ProtocolAsyncTest#TestMultiplePackets. The packetizer does not correctly handle a packet termination at the beginning of a chunk. Test: atest --host bluetooth-vendor-interface-hci-test Bug: 276652301 Change-Id: I1b61efcfa5f817fdb8b5870e81d660be5279fdea --- bluetooth/hci/h4_protocol.cc | 9 +++------ bluetooth/hci/hci_packetizer.cc | 21 +++++++++++++-------- bluetooth/hci/hci_packetizer.h | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/bluetooth/hci/h4_protocol.cc b/bluetooth/hci/h4_protocol.cc index 51a624f4a8..5f6d86e0e2 100644 --- a/bluetooth/hci/h4_protocol.cc +++ b/bluetooth/hci/h4_protocol.cc @@ -105,15 +105,12 @@ void H4Protocol::SendDataToPacketizer(uint8_t* buffer, size_t length) { buffer_offset += 1; } else { bool packet_ready = hci_packetizer_.OnDataReady( - hci_packet_type_, input_buffer, buffer_offset); + hci_packet_type_, input_buffer, &buffer_offset); if (packet_ready) { - // Call packet callback and move offset. - buffer_offset += OnPacketReady(hci_packetizer_.GetPacket()); + // Call packet callback. + OnPacketReady(hci_packetizer_.GetPacket()); // Get ready for the next type byte. hci_packet_type_ = PacketType::UNKNOWN; - } else { - // The data was consumed, but there wasn't a packet. - buffer_offset = input_buffer.size(); } } } diff --git a/bluetooth/hci/hci_packetizer.cc b/bluetooth/hci/hci_packetizer.cc index 5b6c4435cc..41359200b4 100644 --- a/bluetooth/hci/hci_packetizer.cc +++ b/bluetooth/hci/hci_packetizer.cc @@ -51,9 +51,10 @@ const std::vector& HciPacketizer::GetPacket() const { return packet_; } bool HciPacketizer::OnDataReady(PacketType packet_type, const std::vector& buffer, - size_t offset) { + size_t* offset) { bool packet_completed = false; - size_t bytes_available = buffer.size() - offset; + size_t bytes_available = buffer.size() - *offset; + switch (state_) { case HCI_HEADER: { size_t header_size = @@ -62,18 +63,20 @@ bool HciPacketizer::OnDataReady(PacketType packet_type, bytes_remaining_ = header_size; packet_.clear(); } + size_t bytes_to_copy = std::min(bytes_remaining_, bytes_available); - packet_.insert(packet_.end(), buffer.begin() + offset, - buffer.begin() + offset + bytes_to_copy); + packet_.insert(packet_.end(), buffer.begin() + *offset, + buffer.begin() + *offset + bytes_to_copy); bytes_remaining_ -= bytes_to_copy; bytes_available -= bytes_to_copy; + *offset += bytes_to_copy; + if (bytes_remaining_ == 0) { bytes_remaining_ = HciGetPacketLengthForType(packet_type, packet_); if (bytes_remaining_ > 0) { state_ = HCI_PAYLOAD; if (bytes_available > 0) { - packet_completed = - OnDataReady(packet_type, buffer, offset + bytes_to_copy); + packet_completed = OnDataReady(packet_type, buffer, offset); } } else { packet_completed = true; @@ -84,9 +87,10 @@ bool HciPacketizer::OnDataReady(PacketType packet_type, case HCI_PAYLOAD: { size_t bytes_to_copy = std::min(bytes_remaining_, bytes_available); - packet_.insert(packet_.end(), buffer.begin() + offset, - buffer.begin() + offset + bytes_to_copy); + packet_.insert(packet_.end(), buffer.begin() + *offset, + buffer.begin() + *offset + bytes_to_copy); bytes_remaining_ -= bytes_to_copy; + *offset += bytes_to_copy; if (bytes_remaining_ == 0) { state_ = HCI_HEADER; packet_completed = true; @@ -94,6 +98,7 @@ bool HciPacketizer::OnDataReady(PacketType packet_type, break; } } + return packet_completed; } diff --git a/bluetooth/hci/hci_packetizer.h b/bluetooth/hci/hci_packetizer.h index ba3e8412bf..0d9319f1e9 100644 --- a/bluetooth/hci/hci_packetizer.h +++ b/bluetooth/hci/hci_packetizer.h @@ -28,7 +28,7 @@ class HciPacketizer { public: HciPacketizer() = default; bool OnDataReady(PacketType packet_type, const std::vector& data, - size_t offset); + size_t* offset); const std::vector& GetPacket() const; protected: