Hi Stephen, On Tue, May 24, 2022 at 1:26 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > > Both dev_name and short_name are not guaranteed to be NULL terminated so > this instead use strnlen and then attempt to determine if the resulting > string needs to be truncated or not. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216018 > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > --- > net/bluetooth/eir.c | 41 ++++++++++++++++++++++++++--------------- > 1 file changed, 26 insertions(+), 15 deletions(-) > > diff --git a/net/bluetooth/eir.c b/net/bluetooth/eir.c > index 7e930f77ecab..4171edee88e4 100644 > --- a/net/bluetooth/eir.c > +++ b/net/bluetooth/eir.c > @@ -13,6 +13,20 @@ > > #define PNP_INFO_SVCLASS_ID 0x1200 > > +static u8 eir_append_name(u8 *eir, u16 eir_len, u8 type, u8 *data, u8 data_len) > +{ > + u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1]; > + > + /* If data is already NULL terminated just pass it directly */ > + if (data[data_len - 1] == '\0') > + return eir_append_data(eir, eir_len, type, data, data_len); > + > + memcpy(name, data, HCI_MAX_SHORT_NAME_LENGTH); > + name[HCI_MAX_SHORT_NAME_LENGTH] = '\0'; > + > + return eir_append_data(eir, eir_len, type, name, sizeof(name)); > +} > + > u8 eir_append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len) > { > size_t short_len; > @@ -23,29 +37,26 @@ u8 eir_append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len) > return ad_len; > > /* use complete name if present and fits */ > - complete_len = strlen(hdev->dev_name); > + complete_len = strnlen(hdev->dev_name, sizeof(hdev->dev_name)); > if (complete_len && complete_len <= HCI_MAX_SHORT_NAME_LENGTH) > - return eir_append_data(ptr, ad_len, EIR_NAME_COMPLETE, > + return eir_append_name(ptr, ad_len, EIR_NAME_COMPLETE, > hdev->dev_name, complete_len + 1); > > /* use short name if present */ > - short_len = strlen(hdev->short_name); > + short_len = strnlen(hdev->short_name, sizeof(hdev->short_name)); > if (short_len) > - return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, > - hdev->short_name, short_len + 1); > + return eir_append_name(ptr, ad_len, EIR_NAME_SHORT, > + hdev->short_name, > + short_len == HCI_MAX_SHORT_NAME_LENGTH ? > + short_len : short_len + 1); > > /* use shortened full name if present, we already know that name > * is longer then HCI_MAX_SHORT_NAME_LENGTH > */ > - if (complete_len) { > - u8 name[HCI_MAX_SHORT_NAME_LENGTH + 1]; > - > - memcpy(name, hdev->dev_name, HCI_MAX_SHORT_NAME_LENGTH); > - name[HCI_MAX_SHORT_NAME_LENGTH] = '\0'; > - > - return eir_append_data(ptr, ad_len, EIR_NAME_SHORT, name, > - sizeof(name)); > - } > + if (complete_len) > + return eir_append_name(ptr, ad_len, EIR_NAME_SHORT, > + hdev->dev_name, > + HCI_MAX_SHORT_NAME_LENGTH); > > return ad_len; > } > @@ -168,7 +179,7 @@ void eir_create(struct hci_dev *hdev, u8 *data) > u8 *ptr = data; > size_t name_len; > > - name_len = strlen(hdev->dev_name); > + name_len = strnlen(hdev->dev_name, sizeof(hdev->dev_name)); > > if (name_len > 0) { > /* EIR Data type */ > -- > 2.35.1 Here is a tentative fix, I do wonder though why you were trying to set the name directly and not using the likes of bluetoothctl/btmgmt? bluetoothd don't seem to bother setting a shortname so it is probably not reproducible with it but btmgmt does: [mgmt]# name "This is a long name" "Short Name" [mgmt]# @ MGMT Command: Set Local... (0x000f) plen 260 {0x0001} [hci0] 13:37:27.052763 Name: This is a long name Short name: Short Name @ MGMT Event: Command Comp.. (0x0001) plen 263 {0x0001} [hci0] 13:37:27.053224 Set Local Name (0x000f) plen 260 Status: Success (0x00) Name: This is a long name Short name: Short Name Anyway it looks like one needs to be advertising in order to trigger the problem but with the above changes it doesn't crash anymore: @ MGMT Command: Add Extende.. (0x0055) plen 14 {0x0001} [hci0] 13:53:28.130215 Instance: 1 Advertising data length: 3 Flags: 0x06 LE General Discoverable Mode BR/EDR Not Supported Scan response length: 0 < HCI Command: LE Set Exten.. (0x08|0x0037) plen 7 #119 [hci0] 13:53:28.130215 Handle: 0x01 Operation: Complete extended advertising data (0x03) Fragment preference: Minimize fragmentation (0x01) Data length: 0x03 Flags: 0x06 LE General Discoverable Mode BR/EDR Not Supported > HCI Event: Command Complete (0x0e) plen 4 #120 [hci0] 13:53:28.130215 LE Set Extended Advertising Data (0x08|0x0037) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Exte.. (0x08|0x0038) plen 17 #121 [hci0] 13:53:28.130215 Handle: 0x01 Operation: Complete scan response data (0x03) Fragment preference: Minimize fragmentation (0x01) Data length: 0x0d Name (short): Short Name > HCI Event: Command Complete (0x0e) plen 4 #122 [hci0] 13:53:28.130215 LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1 Status: Success (0x00) < HCI Command: LE Set Exten.. (0x08|0x0039) plen 6 #123 [hci0] 13:53:28.130215 Extended advertising: Enabled (0x01) Number of sets: 1 (0x01) Entry 0 Handle: 0x01 Duration: 0 ms (0x00) Max ext adv events: 0 > HCI Event: Command Complete (0x0e) plen 4 #124 [hci0] 13:53:28.134215 LE Set Extended Advertising Enable (0x08|0x0039) ncmd 1 Status: Success (0x00) @ MGMT Event: Command Complete (0x0001) plen 4 {0x0001} [hci0] 13:53:28.134215 Add Extended Advertising Data (0x0055) plen 1 Status: Success (0x00) Instance: 1 -- Luiz Augusto von Dentz
