Begin forwarded message: Date: Mon, 23 May 2022 15:32:38 +0000 From: bugzilla-daemon@xxxxxxxxxx To: stephen@xxxxxxxxxxxxxxxxxx Subject: [Bug 216018] New: Bluetooth: strlen in append_eir_data_to_buf causes calling process to freeze when short/longname does not have double NUL termination. https://bugzilla.kernel.org/show_bug.cgi?id=216018 Bug ID: 216018 Summary: Bluetooth: strlen in append_eir_data_to_buf causes calling process to freeze when short/longname does not have double NUL termination. Product: Networking Version: 2.5 Kernel Version: 5.17.6-1-MANJARO #1 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Other Assignee: stephen@xxxxxxxxxxxxxxxxxx Reporter: tomu1@xxxxxxxxxxxx Regression: No Created attachment 301019 --> https://bugzilla.kernel.org/attachment.cgi?id=301019&action=edit Program to cause strlen Kernel Bug The process I use to set the bluetooth short/longname via HCI freezes with a kernel bug. I have investigated this problem and have possibly found the issue. When writing the bluetooth localname to the kernel via HCI, the userspace is required to write the command to the HCI socket in the following way (https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt): " Command Code: 0x000F Controller Index: <controller id> Command Parameters: Name (249 Octets) Short_Name (11 Octets) Return Parameters: Name (249 Octets) Short_Name (11 Octets)) [...] The values of name and short name will be remembered when switching the controller off and back on again. So the name and short name only have to be set once when a new controller is found and will stay until removed. " It is requested that both Name and Short_Name are zero terminated. When this command is sent via the HCI socket eventually set_local_name is called in the kernel: static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, u16 len) { struct mgmt_cp_set_local_name *cp = data; The structures which are important here are hci_dev and mgmt_cp_set_local_name: struct hci_dev (excerpt): The relevant struct is defined as such: __u8 dev_name[HCI_MAX_NAME_LENGTH]; __u8 short_name[HCI_MAX_SHORT_NAME_LENGTH]; The length macros are defined as such: #define HCI_MAX_NAME_LENGTH 248 #define HCI_MAX_SHORT_NAME_LENGTH 10 struct mgmt_cp_set_local_name { __u8 name[MGMT_MAX_NAME_LENGTH]; __u8 short_name[MGMT_MAX_SHORT_NAME_LENGTH]; } __packed; Its length macros are the following: /* Reserve one extra byte for names in management messages so that they * are always guaranteed to be nul-terminated */ #define MGMT_MAX_NAME_LENGTH (HCI_MAX_NAME_LENGTH + 1) #define MGMT_MAX_SHORT_NAME_LENGTH (HCI_MAX_SHORT_NAME_LENGTH + 1) When set_local_name (net/bluetooth/mgmt.c) is called an memcpy is used to transfer the names: memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); if (!hdev_is_powered(hdev)) { memcpy(hdev->dev_name, cp->name, sizeof(hdev->dev_name)); This would not be a problem in of itself, however when ext_info_changed is called in the same function (and possibly elsewhere), it internally uses append_eir_data_to_buf, which uses strlen to determine the length of the device/shortname: static u16 append_eir_data_to_buf(struct hci_dev *hdev, u8 *eir) ... name_len = strlen(hdev->dev_name); .... name_len = strlen(hdev->short_name); As such, when the userspace gives a name which does not have 2 NUL bytes at the end of long(dev)name and shortname, then the strlen will search over the size of the buffer, as internal structure does not have a NUL byte, which, sadly, causes my started hci process to completely freeze (unrecoverable) due to kernel hardening kicking in. An easy wasy to reproduce this issue is by executing the attached program. Please note blueZ and all of these Bluetooth grabbing services need to be disabled and the controller itself must be powered off. A temporary workaround is to terminate the sent names with two NUL bytes. It would be very kind of you to take a look at this issue. Thank you for your time, Tom Unbehau -- You may reply to this email to add a comment. You are receiving this mail because: You are the assignee for the bug.