linux-kernel - Re: [PATCH] USB: drop misleading usb_set

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]

Date:   Mon, 12 Dec 2022 17:08:17 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Oliver Neukum <oneukum@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: drop misleading usb_set_intfdata() kernel doc

On Mon, Dec 12, 2022 at 10:25:25AM -0500, Alan Stern wrote:

> > Since you insist, I'll just rewrite the whole thing.
> 
> You're both missing the main point, which is that the USB core clears 
> intfdata after a driver is unbound.

I assure you that that hasn't been missed. :)

> As a consequence, drivers don't 
> need to worry about clearing intfdata themselves -- a fact which is 
> _not_ easily apparent from the implementation.  This would be a useful 
> thing to mention in the kerneldoc, as it may help prevent lots of 
> drivers from making unnecessary function calls (like the ones that 
> Vincent recently removed).

My point is that the fact the USB core (and driver core) clears the
pointer after the driver is unbound is mostly irrelevant. The driver
would typically have freed the driver data at that point anyway, and the
interface must no longer be used by the driver (e.g. as it could be
bound to a new driver).

This is a fundamental property of the driver model and not something
that necessarily needs to be repeated for every function that operates
on a struct device (or a subsystem container like struct usb_interface).

> Of course, this doesn't mean that drivers can't clear intfdata if they 
> want to, for example, if they use it as an internal flag.  But 
> developers shouldn't feel that they _need_ to clear it as a sort of 
> hygienic measure.

Right.

> IMO it's worthwhile keeping the kerneldoc (but correcting it) so that it 
> can get this point across.

As you saw I fixed up the kernel doc in v2, and tried to get the point
about not having to clear the pointer across without getting into too
much detail.

The fact that USB core and driver core clears the pointer is an
implementation detail which in principle could change. The important
part is that drivers generally should not touch the struct device or
containing structure after unbind (and must do so with care otherwise).

Johan