Thread (11 messages) 11 messages, 5 authors, 13d ago

Re: [PATCH v2 2/2] Add TAINT_DRIVER_OVERRIDE for usage of driver_override

flat view

From: Uwe Kleine-König <hidden>
Date: 2026-09-28 08:50:53
Also in: driver-core, lkml

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:
quoted
On Sun Sep 27, 2026 at 10:03 AM CEST, Uwe Kleine-König wrote:
quoted
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 <redacted>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Uwe Kleine-König <redacted>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/ (local)
I came up with the idea on my own, but ok, will add that reference.
quoted
quoted
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()?
quoted
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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help