2022-05-24 01:56:06

by Stephen Hemminger

[permalink] [raw]
Subject: Fw: [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.



Begin forwarded message:

Date: Mon, 23 May 2022 15:32:38 +0000
From: [email protected]
To: [email protected]
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: [email protected]
Reporter: [email protected]
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.