Re: [PATCH 1/5] Input: joystick - replace strlcat() with strscpy()

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

Hi Greg,

> Date: 2026-10-17 10:19:59+0200
> From: Greg KH <greg@kroah.com>
>
> On Fri, Oct 16, 2026 at 12:17:05PM -0700, Dmitry Torokhov wrote:
> > Hi Shihuang,
> > 
> > On Sat, Oct 17, 2026 at 01:04:07AM +0800, Shihuang Liu wrote:
> > > The pxrc and xpad drivers build the physical path by appending a
> > > constant "/input0" suffix with strlcat() after usb_make_path().
> > >
> > > In preparation for removing the deprecated strlcat() API[1],
> > > replace it with strnlen() followed by strscpy() into the remainder
> > > of the buffer, as commit 34b228b739f5 ("HID: usbhid: replace
> > > strlcat with better alternatives") did for the usbhid drivers.
> > ...
> > >  	usb_make_path(udev, pxrc->phys, sizeof(pxrc->phys));
> > > -	strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys));
> > > +	len = strnlen(pxrc->phys, sizeof(pxrc->phys));
> > > +	strscpy(pxrc->phys + len, "/input0", sizeof(pxrc->phys) - len);
> > 
> > I really dislike this conversion pattern, and I do not want to see it
> > open-coded across 18 drivers in this series.
> > 
> > usb_make_path() is just a trivial wrapper around snprintf() that already
> > returns the length of the formatted string. Calling usb_make_path(),
> > ignoring its return value, rescanning the buffer with strnlen(), and
> > then open-coding pointer and size arithmetic for strscpy() (along with
> > an extra local variable in every probe function) makes the code much
> > uglier and harder to read than what we had before.
> > 
> > All of these USB input drivers already include <linux/usb/input.h>, and
> > drivers across other subsystems (drivers/hid/usbhid/, drivers/media/rc/,
> > drivers/media/usb/, sound/usb/caiaq/, drivers/usb/storage/onetouch.c)
> > all use the exact same usb_make_path() + suffix pattern ("/input0",
> > "/input%d", "/ir0", "/button", etc.).
> > 
> > If we want to get rid of strlcat() here, please introduce a generic
> > helper in include/linux/usb.h (or a USB input helper in
> > include/linux/usb/input.h) that formats the USB path together with the
> > subpath in a single pass, for example:
> > 
> > __printf(4, 5)
> > int usb_make_subpath(struct usb_device *dev, char *buf, size_t size,
> > 		     const char *fmt, ...);
> > 
> > (or a simple const char *suffix variant if formatting is not needed).
> > Then each driver can do:
> > 
> > 	usb_make_subpath(udev, pxrc->phys, sizeof(pxrc->phys), "input0");
> > 
> > and even drivers/hid/usbhid/hid-core.c can simply do:
> > 
> > 	usb_make_subpath(dev, hid->phys, sizeof(hid->phys),
> > 			 "input%d", intf->altsetting[0].desc.bInterfaceNumber);
> > 
> > instead of repeating this strnlen() + strscpy() boilerplate everywhere.
> 
> I'm all for that too, and putting it in the USB core for everyone to use
> would be great.

You might be interested in <https://lore.kernel.org/linux-hardening/cover.1751747518.git.alx@kernel.org/>.
I was working on something like that, but actually much simpler.
It is meant to replace strscpy()/snprintf() by versions more similar to
Plan9's strecpy(2)/seprint(2), but without some important flaw that the
Plan9 APIs have (about handling NULL).


Have a lovely day!
Alex

> 
> thanks,
> 
> greg k-h
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]