linux-kernel - Re: [PATCH 1/5] nubus: Simplify check in remove callback

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: <4b44c8a4-7d2b-83d-60fa-2d4e4f8284db@linux-m68k.org>
Date:   Sat, 31 Jul 2021 20:32:38 +1000 (AEST)
From:   Finn Thain <fthain@...ux-m68k.org>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-m68k@...ts.linux-m68k.org, linux-kernel@...r.kernel.org,
        kernel@...gutronix.de
Subject: Re: [PATCH 1/5] nubus: Simplify check in remove callback

On Tue, 27 Jul 2021, Uwe Kleine-König wrote:

> On Tue, Jul 27, 2021 at 07:50:39PM +1000, Finn Thain wrote:
> > On Tue, 27 Jul 2021, Uwe Kleine-König wrote:
> > 
> > > The driver core only calls a remove callback when the device was 
> > > successfully bound (aka probed) before. So dev->driver is never 
> > > NULL.
> > 
> > Are you sure dev->driver is non-NULL for the lifetime of the device? A 
> > quick glance at device_reprobe() makes me wonder about that.
> 
> Not for the lifetime of the device, but as long as a driver is bound to 
> the device. device_reprobe() calls device_driver_detach() after which 
> the driver is considered unbound and dev->driver is assigned NULL. (via 
> device_driver_detach -> device_release_driver_internal -> 
> __device_release_driver)
> 

Are you saying that this situation does not presently apply to nubus, 
zorro or superhyway? Or are you saying that the remove callback will never 
be called in this situation?

Perhaps the device_reprobe() API should be documented accordingly, so that 
a missing NULL check can be added if need be. Perhaps it is better to just 
leave this pattern as is (i.e. keep the NULL check).