Trying to register a device on a bus which has not yet been registered
used to trigger a NULL-pointer dereference, but since the const bus
structure rework registration instead succeeds without the device being
added to the bus.

Reject devices with unregistered buses to catch any callers that get
the ordering wrong and to handle bus registration failures more
gracefully.

Fixes: 5221b82d46f2 ("driver core: bus: bus_add/probe/remove_device() cleanups")
Cc: stable@vger.kernel.org	# 6.3
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/base/bus.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 8b6722ff8590..d17bd91490ee 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -544,10 +544,10 @@ static const struct attribute_group driver_override_dev_group = {
  */
 int bus_add_device(struct device *dev)
 {
-	struct subsys_private *sp = bus_to_subsys(dev->bus);
+	struct subsys_private *sp;
 	int error;
 
-	if (!sp) {
+	if (!dev->bus) {
 		/*
 		 * This is a normal operation for many devices that do not
 		 * have a bus assigned to them, just say that all went
@@ -556,6 +556,13 @@ int bus_add_device(struct device *dev)
 		return 0;
 	}
 
+	sp = bus_to_subsys(dev->bus);
+	if (!sp) {
+		pr_err("%s: cannot add device '%s' to unregistered bus '%s'\n",
+		       __func__, dev_name(dev), dev->bus->name);
+		return -EINVAL;
+	}
+
 	/*
 	 * Reference in sp is now incremented and will be dropped when
 	 * the device is removed from the bus
-- 
2.53.0

On Mon Apr 27, 2026 at 12:28 PM CEST, Johan Hovold wrote:
> Trying to register a device on a bus which has not yet been registered
> used to trigger a NULL-pointer dereference, but since the const bus
> structure rework registration instead succeeds without the device being
> added to the bus.
>
> Reject devices with unregistered buses to catch any callers that get
> the ordering wrong and to handle bus registration failures more
> gracefully.
>
> Fixes: 5221b82d46f2 ("driver core: bus: bus_add/probe/remove_device() cleanups")
> Cc: stable@vger.kernel.org	# 6.3

Hm...this sounds like hardening and not like a "real" bug fix. Do you have a
specific reason why you added Cc: stable?

On Tue, Apr 28, 2026 at 09:09:04PM +0200, Danilo Krummrich wrote:
> On Mon Apr 27, 2026 at 12:28 PM CEST, Johan Hovold wrote:
> > Trying to register a device on a bus which has not yet been registered
> > used to trigger a NULL-pointer dereference, but since the const bus
> > structure rework registration instead succeeds without the device being
> > added to the bus.
> >
> > Reject devices with unregistered buses to catch any callers that get
> > the ordering wrong and to handle bus registration failures more
> > gracefully.
> >
> > Fixes: 5221b82d46f2 ("driver core: bus: bus_add/probe/remove_device() cleanups")
> > Cc: stable@vger.kernel.org	# 6.3
> 
> Hm...this sounds like hardening and not like a "real" bug fix. Do you have a
> specific reason why you added Cc: stable?

It's certainly a bug fix and this change in behaviour was clearly
unintended.

Any caller getting the ordering wrong would now succeed in registering
devices, but no driver would ever be bound which is harder to detect
than the earlier crashes. 

Whether any offenders have snuck in since 6.3 I don't know, but I still
think this warrants a backport.

Johan

On Wed Apr 29, 2026 at 12:11 PM CEST, Johan Hovold wrote:
> On Tue, Apr 28, 2026 at 09:09:04PM +0200, Danilo Krummrich wrote:
>> On Mon Apr 27, 2026 at 12:28 PM CEST, Johan Hovold wrote:
>> > Trying to register a device on a bus which has not yet been registered
>> > used to trigger a NULL-pointer dereference, but since the const bus
>> > structure rework registration instead succeeds without the device being
>> > added to the bus.
>> >
>> > Reject devices with unregistered buses to catch any callers that get
>> > the ordering wrong and to handle bus registration failures more
>> > gracefully.
>> >
>> > Fixes: 5221b82d46f2 ("driver core: bus: bus_add/probe/remove_device() cleanups")
>> > Cc: stable@vger.kernel.org	# 6.3
>> 
>> Hm...this sounds like hardening and not like a "real" bug fix. Do you have a
>> specific reason why you added Cc: stable?
>
> It's certainly a bug fix and this change in behaviour was clearly
> unintended.
>
> Any caller getting the ordering wrong would now succeed in registering
> devices, but no driver would ever be bound which is harder to detect
> than the earlier crashes. 
>
> Whether any offenders have snuck in since 6.3 I don't know, but I still
> think this warrants a backport.

I see where you are coming from, and I agree that having an explicit error print
is an improvement over "the device just never got probed".

However, this isn't an actual bug -- it just happens to make a "real" bug less
obvious to catch.

That said, I don't see how this warrants a stable backport, i.e. it doesn't even
fall under the "this could be a problem" or "theoretical bug" category, which
typically are not accepted either.

As mentioned in the other thread, if this was relaxed, I'm happy to hear about
it.

On Wed, Apr 29, 2026 at 01:11:44PM +0200, Danilo Krummrich wrote:
> On Wed Apr 29, 2026 at 12:11 PM CEST, Johan Hovold wrote:
> > On Tue, Apr 28, 2026 at 09:09:04PM +0200, Danilo Krummrich wrote:
> >> On Mon Apr 27, 2026 at 12:28 PM CEST, Johan Hovold wrote:
> >> > Trying to register a device on a bus which has not yet been registered
> >> > used to trigger a NULL-pointer dereference, but since the const bus
> >> > structure rework registration instead succeeds without the device being
> >> > added to the bus.
> >> >
> >> > Reject devices with unregistered buses to catch any callers that get
> >> > the ordering wrong and to handle bus registration failures more
> >> > gracefully.
> >> >
> >> > Fixes: 5221b82d46f2 ("driver core: bus: bus_add/probe/remove_device() cleanups")
> >> > Cc: stable@vger.kernel.org	# 6.3
> >> 
> >> Hm...this sounds like hardening and not like a "real" bug fix. Do you have a
> >> specific reason why you added Cc: stable?
> >
> > It's certainly a bug fix and this change in behaviour was clearly
> > unintended.
> >
> > Any caller getting the ordering wrong would now succeed in registering
> > devices, but no driver would ever be bound which is harder to detect
> > than the earlier crashes. 
> >
> > Whether any offenders have snuck in since 6.3 I don't know, but I still
> > think this warrants a backport.
> 
> I see where you are coming from, and I agree that having an explicit error print
> is an improvement over "the device just never got probed".
> 
> However, this isn't an actual bug -- it just happens to make a "real" bug less
> obvious to catch.

It seems we have differing definitions of "bug", but to me this is
clearly a bug in driver core. Whether anyone will hit it, is a separate
issue.

But if we have any racing subsystem vs device registrations this would
allow us to catch and fix those more easily. And the fix is straight
forward and only turns a silent breakage into a properly logged error.

> That said, I don't see how this warrants a stable backport, i.e. it doesn't even
> fall under the "this could be a problem" or "theoretical bug" category, which
> typically are not accepted either.

Yeah, under the documented rules it may be a bit of a stretch unless you
consider it a "oh, that's not good" issue.

> As mentioned in the other thread, if this was relaxed, I'm happy to hear about
> it.

It has been in practice as I mentioned there.

Johan

On Wed Apr 29, 2026 at 1:33 PM CEST, Johan Hovold wrote:
> It seems we have differing definitions of "bug", but to me this is
> clearly a bug in driver core.

I think we do -- you are saying it is a bug that the driver core does not print
a warning when a caller introduces a bug on its own due to an ordering
violation.

While it clearly is an improvement, I indeed do not consider this a bug.

On Wed, Apr 29, 2026 at 04:52:08PM +0200, Danilo Krummrich wrote:
> On Wed Apr 29, 2026 at 1:33 PM CEST, Johan Hovold wrote:
> > It seems we have differing definitions of "bug", but to me this is
> > clearly a bug in driver core.
> 
> I think we do -- you are saying it is a bug that the driver core does not print
> a warning when a caller introduces a bug on its own due to an ordering
> violation.

No, I'm saying that it's a bug in driver core to silently treat a device
that is registered before its bus as a bus-less device.

Johan

On Wed Apr 29, 2026 at 5:08 PM CEST, Johan Hovold wrote:
> No, I'm saying that it's a bug in driver core to silently treat a device
> that is registered before its bus as a bus-less device.

This is an argument that I can buy into, but in the previous discussion (and in
the commit message) the whole motivation evolved around "reject devices with
unregistered buses to catch any callers that get the ordering wrong", i.e. catch
other people's bugs.

What you are raising now is "the driver core is conflating no bus with
unregistered bus handling". However, the commit message does not reflect that at
all.

Can you please adjust the commit message accordingly?

(As for the stable question, I don't think this changes anything though.)

On Wed, Apr 29, 2026 at 05:29:02PM +0200, Danilo Krummrich wrote:
> On Wed Apr 29, 2026 at 5:08 PM CEST, Johan Hovold wrote:
> > No, I'm saying that it's a bug in driver core to silently treat a device
> > that is registered before its bus as a bus-less device.
> 
> This is an argument that I can buy into, but in the previous discussion (and in
> the commit message) the whole motivation evolved around "reject devices with
> unregistered buses to catch any callers that get the ordering wrong", i.e. catch
> other people's bugs.
>
> What you are raising now is "the driver core is conflating no bus with
> unregistered bus handling". However, the commit message does not reflect that at
> all.

The commit message already explains the issue:

	Trying to register a device on a bus which has not yet been
	registered used to trigger a NULL-pointer dereference, but since
	the const bus structure rework registration instead succeeds
	without the device being added to the bus.

namely that

	 registration [...] succeeds [but] without the device being
	 added to the bus.

> Can you please adjust the commit message accordingly?

Perhaps I can add "(i.e. as if it were a bus-less device)" to stress it
more but I'm not sure it's needed.

Johan

On Wed Apr 29, 2026 at 6:00 PM CEST, Johan Hovold wrote:
> namely that
>
> 	 registration [...] succeeds [but] without the device being
> 	 added to the bus.

[...]

> Perhaps I can add "(i.e. as if it were a bus-less device)" to stress it
> more but I'm not sure it's needed.

I think that would be good, thanks!

Your above quote only vaguely implies that and I think it is pretty hard to
infer that without already being aware of this implication.

It even deserves more emphasis; the current commit message reads as if it's only
about adding an additional warning.

Thanks,
Danilo