HID: usbhid: replace strlcat with better alternatives

In preparation for the removal of the strlcat() API as per the KSPP,
replace the string concatenation logic in hid-core, usbkbd, and
usbmouse with struct seq_buf, which tracks the current write position
and remaining space internally. The changes implemented include:

- Replace device name and phys concatenation with seq_buf_puts().
- Include Struct seq_buf and its initialization.
- Include header file of seq_buf.
- Replace strlen() with seq_buf_used() on the string buffer which was
  tracked by seq_buf to increase speed.
- Add size_t len in files which did not have it.
- Use of strscpy with length in place of strlcat.

Testing: This driver was compiled as a module as well as in-built in
QEMU with the QEMU basic mouse, and QEMU basic keyboard. The testing was
done in the following steps.
- Add Hardware Mouse in QEMU checking the usbhid module.
- Verify dmesg string name of mouse.
- Blacklist hidusb module from auto-loading, and removing the module via
  rmmod.
- Load usbmouse module, and reattach QEMU mouse.
- Verify dmesg string name of mouse.
- Repeat same procedure on usbkbd module.

This aligns the driver with KSPP security guidelines.

Link: https://github.com/KSPP/linux/issues/370
Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index fbbfc0f60829becbf4445d9ffec96ab0ed1d9fc6..4f9d01270c5dac0468e037abd63e0ca506b6252f 100644 (file)
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -27,6 +27,7 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/string.h>
+#include <linux/seq_buf.h>
 
 #include <linux/usb.h>
 
@@ -1366,6 +1367,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
        struct usb_endpoint_descriptor *ep;
        struct usbhid_device *usbhid;
        struct hid_device *hid;
+       struct seq_buf hid_name;
        size_t len;
        int ret;
 
@@ -1397,7 +1399,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
        hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
        hid->product = le16_to_cpu(dev->descriptor.idProduct);
        hid->version = le16_to_cpu(dev->descriptor.bcdDevice);
-       hid->name[0] = 0;
+       seq_buf_init(&hid_name, hid->name, sizeof(hid->name));
        if (intf->cur_altsetting->desc.bInterfaceProtocol ==
                        USB_INTERFACE_PROTOCOL_MOUSE)
                hid->type = HID_TYPE_USBMOUSE;
@@ -1405,22 +1407,23 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
                hid->type = HID_TYPE_USBNONE;
 
        if (dev->manufacturer)
-               strscpy(hid->name, dev->manufacturer, sizeof(hid->name));
+               seq_buf_puts(&hid_name, dev->manufacturer);
 
        if (dev->product) {
                if (dev->manufacturer)
-                       strlcat(hid->name, " ", sizeof(hid->name));
-               strlcat(hid->name, dev->product, sizeof(hid->name));
+                       seq_buf_puts(&hid_name, " ");
+               seq_buf_puts(&hid_name, dev->product);
        }
 
-       if (!strlen(hid->name))
+       if (!seq_buf_used(&hid_name))
                snprintf(hid->name, sizeof(hid->name), "HID %04x:%04x",
                         le16_to_cpu(dev->descriptor.idVendor),
                         le16_to_cpu(dev->descriptor.idProduct));
 
        usb_make_path(dev, hid->phys, sizeof(hid->phys));
-       strlcat(hid->phys, "/input", sizeof(hid->phys));
-       len = strlen(hid->phys);
+       len = strnlen(hid->phys, sizeof(hid->phys));
+       strscpy(hid->phys + len, "/input", sizeof(hid->phys) - len);
+       len = strnlen(hid->phys, sizeof(hid->phys));
        if (len < sizeof(hid->phys) - 1)
                snprintf(hid->phys + len, sizeof(hid->phys) - len,
                         "%d", intf->altsetting[0].desc.bInterfaceNumber);

diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 6b33e6ad0846ee5a29ed9d998fcc7f7528436c88..83d4df0d7a45731c8ce47ed73d65d121236a5358 100644 (file)
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/usb/input.h>
 #include <linux/hid.h>
+#include <linux/seq_buf.h>
 
 /*
  * Version Information
@@ -266,8 +267,10 @@ static int usb_kbd_probe(struct usb_interface *iface,
        struct usb_endpoint_descriptor *endpoint;
        struct usb_kbd *kbd;
        struct input_dev *input_dev;
+       struct seq_buf kbd_name;
        int i, pipe, maxp;
        int error = -ENOMEM;
+       size_t len;
 
        interface = iface->cur_altsetting;
 
@@ -292,24 +295,26 @@ static int usb_kbd_probe(struct usb_interface *iface,
        kbd->usbdev = dev;
        kbd->dev = input_dev;
        spin_lock_init(&kbd->leds_lock);
+       seq_buf_init(&kbd_name, kbd->name, sizeof(kbd->name));
 
        if (dev->manufacturer)
-               strscpy(kbd->name, dev->manufacturer, sizeof(kbd->name));
+               seq_buf_puts(&kbd_name, dev->manufacturer);
 
        if (dev->product) {
                if (dev->manufacturer)
-                       strlcat(kbd->name, " ", sizeof(kbd->name));
-               strlcat(kbd->name, dev->product, sizeof(kbd->name));
+                       seq_buf_puts(&kbd_name, " ");
+               seq_buf_puts(&kbd_name, dev->product);
        }
 
-       if (!strlen(kbd->name))
+       if (!seq_buf_used(&kbd_name))
                snprintf(kbd->name, sizeof(kbd->name),
                         "USB HIDBP Keyboard %04x:%04x",
                         le16_to_cpu(dev->descriptor.idVendor),
                         le16_to_cpu(dev->descriptor.idProduct));
 
        usb_make_path(dev, kbd->phys, sizeof(kbd->phys));
-       strlcat(kbd->phys, "/input0", sizeof(kbd->phys));
+       len = strnlen(kbd->phys, sizeof(kbd->phys));
+       strscpy(kbd->phys + len, "/input0", sizeof(kbd->phys) - len);
 
        input_dev->name = kbd->name;
        input_dev->phys = kbd->phys;

diff --git a/drivers/hid/usbhid/usbmouse.c b/drivers/hid/usbhid/usbmouse.c
index 7cc4f9558e5f21cee9dfb6e42f39ef97e53ff49b..b3b2abeee614ad2fb3297d9cafa7a3b0e5d6ac03 100644 (file)
--- a/drivers/hid/usbhid/usbmouse.c
+++ b/drivers/hid/usbhid/usbmouse.c
@@ -18,6 +18,7 @@
 #include <linux/init.h>
 #include <linux/usb/input.h>
 #include <linux/hid.h>
+#include <linux/seq_buf.h>
 
 /* for apple IDs */
 #ifdef CONFIG_USB_HID_MODULE
@@ -110,8 +111,10 @@ static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_i
        struct usb_endpoint_descriptor *endpoint;
        struct usb_mouse *mouse;
        struct input_dev *input_dev;
+       struct seq_buf mouse_name;
        int pipe, maxp;
        int error = -ENOMEM;
+       size_t len;
 
        interface = intf->cur_altsetting;
 
@@ -140,24 +143,26 @@ static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_i
 
        mouse->usbdev = dev;
        mouse->dev = input_dev;
+       seq_buf_init(&mouse_name, mouse->name, sizeof(mouse->name));
 
        if (dev->manufacturer)
-               strscpy(mouse->name, dev->manufacturer, sizeof(mouse->name));
+               seq_buf_puts(&mouse_name, dev->manufacturer);
 
        if (dev->product) {
                if (dev->manufacturer)
-                       strlcat(mouse->name, " ", sizeof(mouse->name));
-               strlcat(mouse->name, dev->product, sizeof(mouse->name));
+                       seq_buf_puts(&mouse_name, " ");
+               seq_buf_puts(&mouse_name, dev->product);
        }
 
-       if (!strlen(mouse->name))
+       if (!seq_buf_used(&mouse_name))
                snprintf(mouse->name, sizeof(mouse->name),
                         "USB HIDBP Mouse %04x:%04x",
                         le16_to_cpu(dev->descriptor.idVendor),
                         le16_to_cpu(dev->descriptor.idProduct));
 
        usb_make_path(dev, mouse->phys, sizeof(mouse->phys));
-       strlcat(mouse->phys, "/input0", sizeof(mouse->phys));
+       len = strnlen(mouse->phys, sizeof(mouse->phys));
+       strscpy(mouse->phys + len, "/input0", sizeof(mouse->phys) - len);
 
        input_dev->name = mouse->name;
        input_dev->phys = mouse->phys;