From 8bb08d1ca679b4dcf926edf1d94d26e48a22f3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 9 Jun 2018 22:44:25 +0200 Subject: [PATCH] BTReal: Fix unsafe reinterpret_casts Using reinterpret_cast like that is possibly UB. Replace them with structs/memcpy calls where applicable. --- Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp | 70 ++++++++++--------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp index f5d3014f3d..cab096a523 100644 --- a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp +++ b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp @@ -356,15 +356,17 @@ void BluetoothReal::TriggerSyncButtonHeldEvent() void BluetoothReal::WaitForHCICommandComplete(const u16 opcode) { int actual_length; - std::vector buffer(1024); + SHCIEventCommand packet; // Only try 100 transfers at most, to avoid being stuck in an infinite loop for (int tries = 0; tries < 100; ++tries) { - if (libusb_interrupt_transfer(m_handle, HCI_EVENT, buffer.data(), - static_cast(buffer.size()), &actual_length, 20) == 0 && - reinterpret_cast(buffer.data())->event == HCI_EVENT_COMMAND_COMPL && - reinterpret_cast(buffer.data())->Opcode == opcode) + const int ret = libusb_interrupt_transfer(m_handle, HCI_EVENT, reinterpret_cast(&packet), + sizeof(packet), &actual_length, 20); + if (ret == 0 && actual_length == sizeof(packet) && + packet.EventType == HCI_EVENT_COMMAND_COMPL && packet.Opcode == opcode) + { break; + } } } @@ -381,18 +383,19 @@ void BluetoothReal::SendHCIResetCommand() void BluetoothReal::SendHCIDeleteLinkKeyCommand() { const u8 type = LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_INTERFACE; - std::vector packet(sizeof(hci_cmd_hdr_t) + sizeof(hci_delete_stored_link_key_cp)); + struct Payload + { + hci_cmd_hdr_t header; + hci_delete_stored_link_key_cp command; + }; + Payload payload; + payload.header.opcode = HCI_CMD_DELETE_STORED_LINK_KEY; + payload.header.length = sizeof(payload.command); + payload.command.bdaddr = {}; + payload.command.delete_all = true; - auto* header = reinterpret_cast(packet.data()); - header->opcode = HCI_CMD_DELETE_STORED_LINK_KEY; - header->length = sizeof(hci_delete_stored_link_key_cp); - auto* cmd = - reinterpret_cast(packet.data() + sizeof(hci_cmd_hdr_t)); - cmd->bdaddr = {}; - cmd->delete_all = true; - - libusb_control_transfer(m_handle, type, 0, 0, 0, packet.data(), static_cast(packet.size()), - TIMEOUT); + libusb_control_transfer(m_handle, type, 0, 0, 0, reinterpret_cast(&payload), + static_cast(sizeof(payload)), TIMEOUT); } bool BluetoothReal::SendHCIStoreLinkKeyCommand() @@ -407,13 +410,14 @@ bool BluetoothReal::SendHCIStoreLinkKeyCommand() (sizeof(bdaddr_t) + sizeof(linkkey_t)) * static_cast(m_link_keys.size()); std::vector packet(sizeof(hci_cmd_hdr_t) + payload_size); - auto* header = reinterpret_cast(packet.data()); - header->opcode = HCI_CMD_WRITE_STORED_LINK_KEY; - header->length = payload_size; + hci_cmd_hdr_t header{}; + header.opcode = HCI_CMD_WRITE_STORED_LINK_KEY; + header.length = payload_size; + std::memcpy(packet.data(), &header, sizeof(header)); - auto* cmd = - reinterpret_cast(packet.data() + sizeof(hci_cmd_hdr_t)); - cmd->num_keys_write = static_cast(m_link_keys.size()); + hci_write_stored_link_key_cp command{}; + command.num_keys_write = static_cast(m_link_keys.size()); + std::memcpy(packet.data() + sizeof(hci_cmd_hdr_t), &command, sizeof(command)); // This is really ugly, but necessary because of the HCI command structure: // u8 num_keys; @@ -660,21 +664,21 @@ void BluetoothReal::HandleBulkOrIntrTransfer(libusb_transfer* tr) if (tr->status == LIBUSB_TRANSFER_COMPLETED && tr->endpoint == HCI_EVENT) { - const auto* event = reinterpret_cast(tr->buffer); - if (event->event == HCI_EVENT_LINK_KEY_NOTIFICATION) + const u8 event = tr->buffer[0]; + if (event == HCI_EVENT_LINK_KEY_NOTIFICATION) { - const auto* notification = - reinterpret_cast(tr->buffer + sizeof(hci_event_hdr_t)); - + hci_link_key_notification_ep notification; + std::memcpy(¬ification, tr->buffer + sizeof(hci_event_hdr_t), sizeof(notification)); linkkey_t key; - std::copy(std::begin(notification->key), std::end(notification->key), std::begin(key)); - m_link_keys[notification->bdaddr] = key; + std::copy(std::begin(notification.key), std::end(notification.key), std::begin(key)); + m_link_keys[notification.bdaddr] = key; } - else if (event->event == HCI_EVENT_COMMAND_COMPL && - reinterpret_cast(tr->buffer + sizeof(*event))->opcode == - HCI_CMD_RESET) + else if (event == HCI_EVENT_COMMAND_COMPL) { - m_need_reset_keys.Set(); + hci_command_compl_ep complete_event; + std::memcpy(&complete_event, tr->buffer + sizeof(hci_event_hdr_t), sizeof(complete_event)); + if (complete_event.opcode == HCI_CMD_RESET) + m_need_reset_keys.Set(); } }