On Tue, Oct 10, 2023 at 01:36:57PM +0800, Edward AD wrote: > When accessing hdev->name, the actual string length should prevail > > Reported-by: syzbot+c90849c50ed209d77689@xxxxxxxxxxxxxxxxxxxxxxxxx > Fixes: dcda165706b9 ("Bluetooth: hci_core: Fix build warnings") > Signed-off-by: Edward AD <twuufnxlz@xxxxxxxxx> > --- > net/bluetooth/hci_sock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c > index 5e4f718073b7..72abe54c45dd 100644 > --- a/net/bluetooth/hci_sock.c > +++ b/net/bluetooth/hci_sock.c > @@ -488,7 +488,7 @@ static struct sk_buff *create_monitor_event(struct hci_dev *hdev, int event) > ni->type = hdev->dev_type; > ni->bus = hdev->bus; > bacpy(&ni->bdaddr, &hdev->bdaddr); > - memcpy(ni->name, hdev->name, 8); > + memcpy(ni->name, hdev->name, strlen(hdev->name)); Uh, what's going on here? hdev is: struct hci_dev { ... const char *name; ni is: struct hci_mon_new_index { char name[8]; You can't use "strlen" here in the case that "hdev->name" is larger than 8 bytes. Also, why memcpy() and not strscpy()? Is this supposed to be padded out with %NUL bytes? It appears to be sent over the network, so "yes" seems to be the safe answer. Should ni->name be always %NUL terminated? That I can't tell for sure, but I assume "no", because the solution was to explicitly copy all the bytes _except_ the %NUL byte (using strlen). struct hci_mon_new_index's "name" should be marked __nonstring, and instead strtomem_pad() should be used instead of memcpy. -Kees > > opcode = cpu_to_le16(HCI_MON_NEW_INDEX); > break; > -- > 2.25.1 > -- Kees Cook