On Sun, Sep 27, 2026 at 12:03:54PM +0200, Danilo Krummrich wrote:
> On Sun Sep 27, 2026 at 11:55 AM CEST, Danilo Krummrich wrote:
> > On Sun Sep 27, 2026 at 10:03 AM CEST, Uwe Kleine-König wrote:
> >> Commit fcbfaffee51a ("driver core: add TAINT_FORCED_BIND for when
> >> userspace manually messes with devices and drivers") introduced a taint
> >> for usage of bind/unbind sysfs files that manually trigger driver probe
> >> and remove respectively.
> >>
> >> For drivers that do their resource management correctly (which is also
> >> needed for module unloading) bind and unbind for matching devices are
> >> not critical operations. The thing that makes bind and unbind unsafe is
> >> that drivers can be forced on devices that originally don't match using
> >> driver_override. The result is that e.g. of_device_get_match_data()
> >> returns NULL despite all .of_match_table entries having a non-NULL
> >> .driver_data member which yields a NULL pointer exception for several
> >> drivers. And given that after setting a driver_override a manual bind is
> >> only one way a driver can be bound to an unexpected device, a separate
> >> taint for such an override is justified.
> >>
> >> Reviewed-by: Bradley Morgan <[email protected]>
> >> Reviewed-by: Armin Wolf <[email protected]>
> >> Signed-off-by: Uwe Kleine-König <[email protected]>
> >
> > Suggested-by: Danilo Krummrich <[email protected]>
> > Link:
> > https://lore.kernel.org/driver-core/[email protected]/
I came up with the idea on my own, but ok, will add that reference.
> >> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> >> index c51ad96d4de4..7d5dc016a457 100644
> >> --- a/drivers/base/bus.c
> >> +++ b/drivers/base/bus.c
> >> @@ -513,6 +513,7 @@ static ssize_t driver_override_store(struct device
> >> *dev,
> >> {
> >> int ret;
> >>
> >> + add_taint_module(NULL, TAINT_DRIVER_OVERRIDE, LOCKDEP_STILL_OK);
> >> ret = __device_set_driver_override(dev, buf, count);
> >
> > There are buses (such as SPI) which unfortunately have to call
> > __device_set_driver_override() directly.
> >
> > I think it would be better to move the taint into
> > __device_set_driver_override()
> > and properly document the purpose of __device_set_driver_override().
>
> Of course I meant to say to create a new forwarding function for this purpose,
> such that we do not taint for device_set_driver_override().
What is the rationale to exclude device_set_driver_override()?
For the dynamic spi device creation I like it to trigger the taint. For
sound/soc/samsung/i2s.c it looks as if device_set_driver_override() is
just the lazy way to make the created device bind and there is no reason
to stick to normal binding. And why does it call device_attach()?
Shouldn't that trigger automatically after platform_device_add()?
Also in drivers/slimbus/qcom-ngd-ctrl.c the call to
device_set_driver_override() seems redundant.
> Maybe device_store_driver_override() or device_set_driver_override_store()?
>
> > It only exists as SPI and AP are a bit special; both print "\n" when
> > driver_override is not set, whereas all other buses (and thus the
> > driver-core)
> > produce "(null)\n" in this case. I.e. it should never get any new users.
I guess it's API and thus hardly changable, but I like "\n" better, and
if it's only because "(null)" might be a driver name and there is no way
to distinguish the situation after
echo '(null)' > driver_override
from the normal state.
Best regards
Uwe
signature.asc
Description: PGP signature
