linux-kernel - Re: [PATCH] driver core: faux: Check device binding state by dedicated API device_is

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: <2025030704-lapping-gurgle-d101@gregkh>
Date: Fri, 7 Mar 2025 15:04:56 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
	Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH] driver core: faux: Check device binding state by
 dedicated API device_is_bound()

On Fri, Mar 07, 2025 at 08:47:16PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@...cinc.com>
> 
> Use dedicated API device_is_bound() instead of 'dev->driver' to check
> device binding state in faux_device_create_with_groups().
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---
>  drivers/base/faux.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> index 407c1d1aad50b7969a6dab9d2027d8beab66a754..76fe494a128254aaaf1339386ab37817a732781f 100644
> --- a/drivers/base/faux.c
> +++ b/drivers/base/faux.c
> @@ -154,7 +154,8 @@ struct faux_device *faux_device_create_with_groups(const char *name,
>  	 * if not, let's fail the creation as trying to guess if probe was
>  	 * successful is almost impossible to determine by the caller.
>  	 */
> -	if (!dev->driver) {
> +	/* Do not need to device_lock(dev) due to no race here */
> +	if (!device_is_bound(dev)) {

Ick, this feels wrong.  This is the driver core code, it can poke in
dev->driver if it wants to, and as the lock is not held, and there is no
lock even mentioned in this driver, the comment is confusing unless you
dig and go read that device_is_bound() requires this.

Also, when we start to add locking requirements to functions like
device_is_bound() (which we really should) this call will fail the
check, right?  How are you going to explain that?  :)

thanks,

greg k-h