Re: [PATCH] Input: uinput/uhid - disallow control characters in phys paths

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Aug 03, 2026 at 09:58:35PM +0200, Jiri Kosina wrote:
> On Fri, 24 Jul 2026, David Rheinsberg wrote:
> 
> > > There is no good reason to support those, no physical device will ever
> > > produce those. Allowing \n in phys previously triggered CVE-2026-50292
> > > in libinput - there the PHYS udev property value was used as part of
> > > another udev property value. The linebreak then caused the property
> > > to be split across two lines, allowing uinput devices to inject
> > > malicious properties. While the bug is squarely inlibinput's court
> > > there still isn't a good reason for control characters in uinput/uhid.
> > >
> > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> > > ---
> > >  drivers/hid/uhid.c          |  1 +
> > >  drivers/input/misc/uinput.c |  1 +
> > >  include/linux/input.h       | 15 +++++++++++++++
> > >  3 files changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> > > index 37b60c3aaf66..baf1fe8290f7 100644
> > > --- a/drivers/hid/uhid.c
> > > +++ b/drivers/hid/uhid.c
> > > @@ -513,16 +513,17 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> > >  		ret = PTR_ERR(hid);
> > >  		goto err_free;
> > >  	}
> > > 
> > >  	BUILD_BUG_ON(sizeof(hid->name) != sizeof(ev->u.create2.name));
> > >  	strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
> > >  	BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
> > >  	strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
> > > +	input_sanitize_phys(hid->phys);
> > >  	BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
> > >  	strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
> > > 
> > >  	hid->ll_driver = &uhid_hid_driver;
> > >  	hid->bus = ev->u.create2.bus;
> > >  	hid->vendor = ev->u.create2.vendor;
> > >  	hid->product = ev->u.create2.product;
> > >  	hid->version = ev->u.create2.version;
> > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> > > index d32fa4b508fc..70fe4f3e73bf 100644
> > > --- a/drivers/input/misc/uinput.c
> > > +++ b/drivers/input/misc/uinput.c
> > > @@ -998,16 +998,17 @@ static long uinput_ioctl_handler(struct file 
> > > *file, unsigned int cmd,
> > > 
> > >  		phys = strndup_user(p, 1024);
> > >  		if (IS_ERR(phys)) {
> > >  			retval = PTR_ERR(phys);
> > >  			goto out;
> > >  		}
> > > 
> > >  		kfree(udev->dev->phys);
> > > +		input_sanitize_phys(phys);
> > >  		udev->dev->phys = phys;
> > >  		goto out;
> > > 
> > >  	case UI_BEGIN_FF_UPLOAD:
> > >  		retval = uinput_ff_upload_from_user(p, &ff_up);
> > >  		if (retval)
> > >  			goto out;
> > > 
> > > diff --git a/include/linux/input.h b/include/linux/input.h
> > > index 76f7aa226202..6c182f5c783f 100644
> > > --- a/include/linux/input.h
> > > +++ b/include/linux/input.h
> > > @@ -527,16 +527,31 @@ int input_set_keycode(struct input_dev *dev,
> > > 
> > >  bool input_match_device_id(const struct input_dev *dev,
> > >  			   const struct input_device_id *id);
> > > 
> > >  void input_enable_softrepeat(struct input_dev *dev, int delay, int period);
> > > 
> > >  bool input_device_enabled(struct input_dev *dev);
> > > 
> > > +/**
> > > + * input_sanitize_phys - replace invalid characters in a phys string
> > > + * @phys: the phys path to sanitize (modified in place)
> > > + *
> > > + * Replaces any control characters and non-ASCII characters with '?'.
> > > + **/
> > > +static inline void input_sanitize_phys(char *phys)
> > > +{
> > > +	char *p;
> > > +
> > > +	for (p = phys; *p; p++)
> > > +		if (*p < 0x20 || *p > 0x7e)
> > > +			*p = '?';
> > > +}
> > > +
> > 
> > Reviewed-by: David Rheinsberg <david@readahead.eu>
> > 
> > I would also be fine to just reject them in uinput, but I guess this is the less intrusive option.
> 
> Thanks for the fix.
> 
> Dmitry, can you please Ack the above addition to input.h?

Jiri, as I mentioned I believe that if we need to sanitize phys we
should also sanitize other fields, especially given that they can be
set by userspace.

I also wonder why it needs to be inline? And what is wrong with using
isprint() here? 

Thanks.

-- 
Dmitry