linux-kernel - Re: [PATCH v3] driver core: fix use-after-free of driver_override via driver_match

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DFNEXNI1HK24.PQWIBQEW4IQ4@kernel.org>
Date: Tue, 13 Jan 2026 12:08:58 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Gui-Dong Han" <hanguidong02@...il.com>
Cc: <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
 <linux-kernel@...r.kernel.org>, <baijiaju1990@...il.com>, "Qiu-ji Chen"
 <chenqiuji666@...il.com>
Subject: Re: [PATCH v3] driver core: fix use-after-free of driver_override
 via driver_match_device()

On Tue Jan 13, 2026 at 10:55 AM CET, Danilo Krummrich wrote:
> On Thu Nov 27, 2025 at 3:57 PM CET, Gui-Dong Han wrote:
>> diff --git a/drivers/base/base.h b/drivers/base/base.h
>> index 86fa7fbb3548..72791125de91 100644
>> --- a/drivers/base/base.h
>> +++ b/drivers/base/base.h
>> @@ -166,6 +166,9 @@ void device_set_deferred_probe_reason(const struct device *dev, struct va_format
>>  static inline int driver_match_device(const struct device_driver *drv,
>>  				      struct device *dev)
>>  {
>> +	/* Protects against driver_set_override() races */
>> +	device_lock_assert(dev);
>> +
>>  	return drv->bus->match ? drv->bus->match(dev, drv) : 1;
>>  }
>
> I am not convinced that this is the correct fix, since
>
>   1. Not all match() callbacks access the driver_override field,
>
>   2. driver_override is accessed in other places as well,
>
>   3. driver_override is a bus device specific field (with a common
>      helper admittedly).
>
> I think it would be better to make driver_override a field in the base
> struct device. This way we can not only provide driver_set_override(), but also
> driver_get_override(), which should contain the device_lock_assert() instead.

We should probably avoid the term 'get' in the accessor, since it could be
misleading.

More generally the naming is a bit odd: driver_set_override() reads as if we'd
set an override field of some driver structure, whereas we actually set the
driver_override field of a device structure.

Hence, I'd suggest to eventually go with dev_set_driver_override() and
dev_access_driver_override() (or just dev_driver_override()).

Also, the documentation of the accessor should document that the returned
pointer is only valid as long as the lock is held.

(In Rust it would be possible to make the compiler ensure that the returned
pointer can't be accessed anymore after the lock is dropped.)

> While not all devices require the driver_override field, an additional pointer
> in struct device does not hurt and it clarifies ownership and hence locking.
>
> - Danilo