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

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

[Uwe Kleine-König]

Hello,

the changes since (implicit) v1 that is available at
https://lore.kernel.org/driver-core/20260925182041.1025371-2-u.kleine-koenig@baylibre.com

 - new patch fixing an already existing tiny issue that Sashiko found
 - Fix a build failure that I found before sending v1, but failed to
   squash into the patch
 - Added review tags by Bradley Morgan and Armin Wolf

IMHO this taint is superior to TAINT_FORCED_BIND, but I hesitate to drop
the forced-bind one. On the other hand it might be better to drop it now
(before hitting a mainline release) to not burn the used letter to
indicate that taint?!

Best regards
Uwe

Uwe Kleine-König (2):
  docs: admin-guide: Handle TAINT_FORCED_BIND when parsing
    /proc/sys/kernel/tainted
  Add TAINT_DRIVER_OVERRIDE for usage of driver_override

 Documentation/admin-guide/tainted-kernels.rst | 6 +++++-
 drivers/base/bus.c                            | 1 +
 include/linux/panic.h                         | 3 ++-
 include/trace/events/module.h                 | 3 ++-
 kernel/panic.c                                | 3 ++-
 tools/debugging/kernel-chktaint               | 8 ++++++++
 6 files changed, 20 insertions(+), 4 deletions(-)


base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
-- 
2.47.3

[PATCH v2 1/2] docs: admin-guide: Handle TAINT_FORCED_BIND when parsing /proc/sys/kernel/tainted

[Uwe Kleine-König]

tainted-kernels.rst contains a small script to check which taint bits
are set in /proc/sys/kernel/tainted. Add one more loop iteration to also
handle the newly added TAINT_FORCED_BIND bit.

Also simplify by starting the loop at 0 and save substracting 1 for each
usage of the loop counter.

Fixes: fcbfaffee51a ("driver core: add TAINT_FORCED_BIND for when userspace manually messes with devices and drivers")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

feel free to squash this into fcbfaffee51a without author attribution if
that is considered a good idea.

Best regards
Uwe

 Documentation/admin-guide/tainted-kernels.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
index abbf5e3dd749..a208811ad525 100644
--- a/Documentation/admin-guide/tainted-kernels.rst
+++ b/Documentation/admin-guide/tainted-kernels.rst
@@ -74,7 +74,7 @@ a particular type of taint. It's best to leave that to the aforementioned
 script, but if you need something quick you can use this shell command to check
 which bits are set::
 
-	$ for i in $(seq 20); do echo $(($i-1)) $(($(cat /proc/sys/kernel/tainted)>>($i-1)&1));done
+	$ for i in $(seq 0 20); do echo $i $(($(cat /proc/sys/kernel/tainted)>>$i&1));done
 
 Table for decoding tainted state
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- 
2.47.3

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

[Uwe Kleine-König]

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 <brads@mainlining.org>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 Documentation/admin-guide/tainted-kernels.rst | 6 +++++-
 drivers/base/bus.c                            | 1 +
 include/linux/panic.h                         | 3 ++-
 include/trace/events/module.h                 | 3 ++-
 kernel/panic.c                                | 3 ++-
 tools/debugging/kernel-chktaint               | 8 ++++++++
 6 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
index a208811ad525..9ccac96b1f75 100644
--- a/Documentation/admin-guide/tainted-kernels.rst
+++ b/Documentation/admin-guide/tainted-kernels.rst
@@ -74,7 +74,7 @@ a particular type of taint. It's best to leave that to the aforementioned
 script, but if you need something quick you can use this shell command to check
 which bits are set::
 
-	$ for i in $(seq 0 20); do echo $i $(($(cat /proc/sys/kernel/tainted)>>$i&1));done
+	$ for i in $(seq 0 21); do echo $i $(($(cat /proc/sys/kernel/tainted)>>$i&1));done
 
 Table for decoding tainted state
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -103,6 +103,7 @@ Bit  Log  Number   Reason that got the kernel tainted
  18  _/N   262144  an in-kernel test has been run
  19  _/J   524288  userspace used a mutating debug operation in fwctl
  20  _/Y  1048576  device was manually bound or unbound from a driver
+ 21  _/Z  2097152  a driver was forced on a non-matching device
 ===  ===  =======  ========================================================
 
 Note: The character ``_`` is representing a blank in this table to make reading
@@ -193,3 +194,6 @@ More detailed explanation for tainting
 
  20) ``Y`` If userspace wrote to the `bind` or `unbind` sysfs files and
      successfully bound or removed a device from a driver.
+
+ 21) ``Z`` If userspace wrote to a `driver_override` sysfs file opening the gate
+     for unexpected driver binding.
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);
 	if (ret)
 		return ret;
diff --git a/include/linux/panic.h b/include/linux/panic.h
index 23976b1dfdb6..e6e24d8afcf7 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -90,7 +90,8 @@ static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
 #define TAINT_TEST			18
 #define TAINT_FWCTL			19
 #define TAINT_FORCED_BIND		20
-#define TAINT_FLAGS_COUNT		21
+#define TAINT_DRIVER_OVERRIDE		21
+#define TAINT_FLAGS_COUNT		22
 #define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
 
 struct taint_flag {
diff --git a/include/trace/events/module.h b/include/trace/events/module.h
index 19df3e39bba4..c7cdb1f53bc6 100644
--- a/include/trace/events/module.h
+++ b/include/trace/events/module.h
@@ -27,7 +27,8 @@ struct module;
 	{ (1UL << TAINT_FORCED_MODULE),		"F" },		\
 	{ (1UL << TAINT_CRAP),			"C" },		\
 	{ (1UL << TAINT_UNSIGNED_MODULE),	"E" },		\
-	{ (1UL << TAINT_FORCED_BIND),		"Y" })
+	{ (1UL << TAINT_FORCED_BIND),		"Y" },		\
+	{ (1UL << TAINT_DRIVER_OVERRIDE),	"Z" })
 
 TRACE_EVENT(module_load,
 
diff --git a/kernel/panic.c b/kernel/panic.c
index b824b68fcb08..f5476a61f6f3 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -826,6 +826,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	TAINT_FLAG(TEST,			'N', ' '),
 	TAINT_FLAG(FWCTL,			'J', ' '),
 	TAINT_FLAG(FORCED_BIND,			'Y', ' '),
+	TAINT_FLAG(DRIVER_OVERRIDE,		'Z', ' '),
 };
 
 #undef TAINT_FLAG
@@ -862,7 +863,7 @@ static void print_tainted_seq(struct seq_buf *s, bool verbose)
  * exact size is allocated dynamically; the initial buffer remains
  * as a fallback if allocation fails.
  *
- * The verbose taint string currently requires up to 344 characters.
+ * The verbose taint string currently requires up to 364 characters.
  */
 #define INIT_TAINT_BUF_MAX 370
 
diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
index d8628be37214..14d8febd6b16 100755
--- a/tools/debugging/kernel-chktaint
+++ b/tools/debugging/kernel-chktaint
@@ -219,6 +219,14 @@ else
 	addout "Y"
 	echo " * device was manually bound or unbound from a driver (#20)"
 fi
+
+T=`expr $T / 2`
+if [ `expr $T % 2` -eq 0 ]; then
+	addout " "
+else
+	addout "Z"
+	echo " * a driver was forced on a non-matching device (#21)"
+fi
 echo "Raw taint value as int/string: $taint/'$out'"
 
 # report on any tainted loadable modules
-- 
2.47.3

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

[Danilo Krummrich]

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 <brads@mainlining.org>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/

> 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().

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.

Thanks,
Danilo

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

[Danilo Krummrich]

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 <brads@mainlining.org>
>> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
>
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/
>
>> 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().

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.
>
> Thanks,
> Danilo

Re: [PATCH v2 1/2] docs: admin-guide: Handle TAINT_FORCED_BIND when parsing /proc/sys/kernel/tainted

[Randy Dunlap]

On 9/27/26 1:03 AM, Uwe Kleine-König wrote:
> tainted-kernels.rst contains a small script to check which taint bits
> are set in /proc/sys/kernel/tainted. Add one more loop iteration to also
> handle the newly added TAINT_FORCED_BIND bit.
> 
> Also simplify by starting the loop at 0 and save substracting 1 for each
> usage of the loop counter.
> 
> Fixes: fcbfaffee51a ("driver core: add TAINT_FORCED_BIND for when userspace manually messes with devices and drivers")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

thanks.

> ---
> Hello,
> 
> feel free to squash this into fcbfaffee51a without author attribution if
> that is considered a good idea.
> 
> Best regards
> Uwe
> 
>  Documentation/admin-guide/tainted-kernels.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
> index abbf5e3dd749..a208811ad525 100644
> --- a/Documentation/admin-guide/tainted-kernels.rst
> +++ b/Documentation/admin-guide/tainted-kernels.rst
> @@ -74,7 +74,7 @@ a particular type of taint. It's best to leave that to the aforementioned
>  script, but if you need something quick you can use this shell command to check
>  which bits are set::
>  
> -	$ for i in $(seq 20); do echo $(($i-1)) $(($(cat /proc/sys/kernel/tainted)>>($i-1)&1));done
> +	$ for i in $(seq 0 20); do echo $i $(($(cat /proc/sys/kernel/tainted)>>$i&1));done
>  
>  Table for decoding tainted state
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-- 
~Randy

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

[Greg Kroah-Hartman]

On Sun, Sep 27, 2026 at 11:55:39AM +0200, 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 <brads@mainlining.org>
> > Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> 
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/
> 
> > 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.

Huh?  That feels wrong, can't we fix s390 and spi instead?  Ok, maybe
not s390, but why is spi doing that?

thanks,

greg k-h

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

[Danilo Krummrich]

On Sun Sep 27, 2026 at 6:50 PM CEST, Greg Kroah-Hartman wrote:
> Huh?  That feels wrong, can't we fix s390 and spi instead?  Ok, maybe
> not s390, but why is spi doing that?

Solely to preserve SPI's existing output; when driver_override is unset, SPI has
always printed "\n", whereas most other buses print "(null)\n".

I.e. SPI previously did

	sysfs_emit(buf, "%s\n", spi->driver_override ? : "");

now it does

	sysfs_emit(buf, "%s\n", dev->driver_override.name ?: "");

and the driver core does

	sysfs_emit(buf, "%s\n", dev->driver_override.name);

The driver-core behavior matches most other buses (except SPI and AP).
Switching SPI to it would have changed the userspace-visible value.

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3557 bytes --]

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 <brads@mainlining.org>
> >> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> >
> > Suggested-by: Danilo Krummrich <dakr@kernel.org>
> > Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Danilo Krummrich]

On Mon Sep 28, 2026 at 10:50 AM CEST, Uwe Kleine-König wrote:
> 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 <brads@mainlining.org>
>> >> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
>> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
>> >
>> > Suggested-by: Danilo Krummrich <dakr@kernel.org>
>> > Link: https://lore.kernel.org/driver-core/DLIL9H50MALI.3JROXYEEUM3KU@kernel.org/
>
> I came up with the idea on my own, but ok, will add that reference.

Sure, I didn't mean to suggest you couldn't have arrived at it independently. I
offered the tag because I'd proposed it in the earlier discussion and you agreed
with it there.

>> >> 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()?

It's used / abused within subsystems to hardwire stuff, which should indeed be
fixed, but also doesn't justify the taint, as it is not userspace controlling
it.

> For the dynamic spi device creation I like it to trigger the taint.

Oh, there's a new usage in new_device_store() as of v7.3-rc1. I wasn't aware of
this. I'd just make it call the device_set_driver_override_store() function
then, as it is exactly the same class of issue.

> 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.

I would have preferred "\n" too, but I don't want to mess with this uAPI. When I
implemented the driver_override generalization I looked it up (e.g. in [1]) and
tools seem to care about this. driverctl handles both cases, as it is bus
agnostic, but I don't know what else exists.

[1] https://gitlab.com/driverctl/driverctl/-/blob/0.121/driverctl?ref_type=tags#L99