Re: [PATCH 02/17] Input: serio - attach manual drivers through the driver core

[Leon Romanovsky <leon@kernel.org>]

On Thu, Nov 05, 2026 at 12:34:53PM -0800, Dmitry Torokhov wrote:
> On Thu, Nov 05, 2026 at 09:19:40PM +0200, Leon Romanovsky wrote:
> > On Thu, Nov 05, 2026 at 09:05:29AM -0800, Dmitry Torokhov wrote:
> > > On Thu, Nov 05, 2026 at 01:42:15PM +0200, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@nvidia.com>
> > > > 
> > > > Manual serio binding connects a driver before driver core can admit the
> > > > device. A rejected binding therefore arrives after functional setup.
> > > > 
> > > > Keep the port ID check and use device_driver_attach() for the existing bus
> > > > probe path. Driver core then serializes admission and handles probe failure
> > > > before publishing the binding.
> > > > 
> > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > > ---
> > > >  drivers/input/serio/serio.c | 24 +++---------------------
> > > >  1 file changed, 3 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> > > > index 54dd26249b027..604440244c1ca 100644
> > > > --- a/drivers/input/serio/serio.c
> > > > +++ b/drivers/input/serio/serio.c
> > > > @@ -81,28 +81,10 @@ static int serio_match_port(const struct serio_device_id *ids, struct serio *ser
> > > >  
> > > >  static int serio_bind_driver(struct serio *serio, struct serio_driver *drv)
> > > >  {
> > > > -	int error;
> > > > -
> > > > -	if (serio_match_port(drv->id_table, serio)) {
> > > > -
> > > > -		serio->dev.driver = &drv->driver;
> > > > -		if (serio_connect_driver(serio, drv)) {
> > > > -			serio->dev.driver = NULL;
> > > > -			return -ENODEV;
> > > > -		}
> > > > +	if (!serio_match_port(drv->id_table, serio))
> > > > +		return 0;
> > > >  
> > > > -		error = device_bind_driver(&serio->dev);
> > > > -		if (error) {
> > > > -			dev_warn(&serio->dev,
> > > > -				 "device_bind_driver() failed for %s (%s) and %s, error: %d\n",
> > > > -				 serio->phys, serio->name,
> > > > -				 drv->description, error);
> > > > -			serio_disconnect_driver(serio);
> > > > -			serio->dev.driver = NULL;
> > > > -			return error;
> > > > -		}
> > > > -	}
> > > > -	return 0;
> > > > +	return device_driver_attach(&drv->driver, &serio->dev);
> > > 
> > > I am not sure how safe it is, serio (and gameport) have pretty elaborate
> > > structure and locking rules because they can recurse into themselves (to
> > > support case of Synaptics touch-pads with pass-through PS/2 ports).
> > > 
> > > But this call is for user-initiated probing anyways, I am not sure if it
> > > even needs "trust" handling.
> > 
> > This patch does not change the locking; the lock was already held on paths such
> > as the rescan path. This change makes that explicit.
> 
> Does this enforce synchronous probing?

No, it ensures that a device is bound to only one driver at a time,
without races.

> 
> Anyway, I would like to get rid of this special probing in serio core.
> Given that the code path is only triggered by explicit user request I do
> not believe we need to worry about supporting trust notion here, so
> please drop it from the series.

Technically, this is exactly the flow we should be concerned about. Users
attempting to load the driver generally don't know whether their device is
trusted; attestation software is responsible for determining that. I modified
serio to ensure that driver attachment happens under the driver core lock.

One idea I had over the weekend was to introduce a special API for driver
attachment that says, "I don't care." It would allow attachment only in
"full" mode, as it does now, and fail in any other trust mode.

Thanks.
> 
> Thanks.
> 
> -- 
> Dmitry
>