On Mon, Oct 19, 2026 at 09:51:37AM +0200, Alejandro Colomar wrote:
> Hi Greg,
> 
> > Date: 2026-10-17 10:19:59+0200
> > From: Greg KH <[email protected]>
> >
> > 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/[email protected]/>.
> 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).

Look at the patches from Kees on the hardening list for the seqbuf
stuff, that ideally will replace these types of string functions
entirely over time.

thanks,

greg k-h

Reply via email to