From: Oliver Neukum <oneukum@suse.com>
To: Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
Marc Kleine-Budde <mkl@pengutronix.de>,
linux-can@vger.kernel.org
Cc: "Wolfgang Grandegger" <wg@grandegger.com>,
"David S . Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Frank Jungclaus" <frank.jungclaus@esd.eu>,
socketcan@esd.eu, "Yasushi SHOJI" <yashi@spacecubics.com>,
"Stefan Mätje" <stefan.maetje@esd.eu>,
"Hangyu Hua" <hbh25y@gmail.com>,
"Oliver Hartkopp" <socketcan@hartkopp.net>,
"Peter Fink" <pfink@christ-es.de>,
"Jeroen Hofstee" <jhofstee@victronenergy.com>,
"Christoph Möhring" <cmoehring@christ-es.de>,
"John Whittington" <git@jbrengineering.co.uk>,
"Vasanth Sadhasivan" <vasanth.sadhasivan@samsara.com>,
"Jimmy Assarsson" <extja@kvaser.com>,
"Anssi Hannula" <anssi.hannula@bitwise.fi>,
"Pavel Skripkin" <paskripkin@gmail.com>,
"Stephane Grosjean" <s.grosjean@peak-system.com>,
"Wolfram Sang" <wsa+renesas@sang-engineering.com>,
"Gustavo A . R . Silva" <gustavoars@kernel.org>,
"Julia Lawall" <Julia.Lawall@inria.fr>,
"Dongliang Mu" <dzm91@hust.edu.cn>,
"Sebastian Haas" <haas@ems-wuensche.com>,
"Maximilian Schneider" <max@schneidersoft.net>,
"Daniel Berglund" <db@kvaser.com>,
"Olivier Sobrie" <olivier@sobrie.be>,
"Remigiusz Kołłątaj" <remigiusz.kollataj@mobica.com>,
"Jakob Unterwurzacher"
<jakob.unterwurzacher@theobroma-systems.com>,
"Martin Elshuber" <martin.elshuber@theobroma-systems.com>,
"Philipp Tomsich" <philipp.tomsich@theobroma-systems.com>,
"Bernd Krumboeck" <b.krumboeck@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
"Alan Stern" <stern@rowland.harvard.edu>,
linux-usb@vger.kernel.org
Subject: Re: [PATCH 0/8] can: usb: remove all usb_set_intfdata(intf, NULL) in drivers' disconnect()
Date: Mon, 5 Dec 2022 09:35:46 +0100 [thread overview]
Message-ID: <9493232b-c8fa-5612-fb13-fccf58b01942@suse.com> (raw)
In-Reply-To: <20221203133159.94414-1-mailhol.vincent@wanadoo.fr>
On 03.12.22 14:31, Vincent Mailhol wrote:
> The core sets the usb_interface to NULL in [1]. Also setting it to
> NULL in usb_driver::disconnects() is at best useless, at worse risky.
Hi,
I am afraid there is a major issue with your series of patches.
The drivers you are removing this from often have a subsequent check
for the data they got from usb_get_intfdata() being NULL.
That pattern is taken from drivers like btusb or CDC-ACM, which
claim secondary interfaces disconnect() will be called a second time
for.
In addition, a driver can use setting intfdata to NULL as a flag
for disconnect() having proceeded to a point where certain things
can no longer be safely done. You need to check for that in every driver
you remove this code from and if you decide that it can safely be removed,
which is likely, then please also remove checks like this:
struct ems_usb *dev = usb_get_intfdata(intf);
usb_set_intfdata(intf, NULL);
if (dev) {
unregister_netdev(dev->netdev);
Either it can be called a second time, then you need to leave it
as is, or the check for NULL is superfluous. But only removing setting
the pointer to NULL never makes sense.
Regards
Oliver
next prev parent reply other threads:[~2022-12-05 8:35 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-03 13:31 [PATCH 0/8] can: usb: remove all usb_set_intfdata(intf, NULL) in drivers' disconnect() Vincent Mailhol
2022-12-03 13:31 ` [PATCH 1/8] can: ems_usb: ems_usb_disconnect(): fix NULL pointer dereference Vincent Mailhol
2022-12-03 13:31 ` [PATCH 2/8] can: esd_usb: esd_usb_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 3/8] can: gs_usb: gs_usb_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 4/8] can: kvaser_usb: kvaser_usb_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 5/8] can: mcba_usb: mcba_usb_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 6/8] can: ucan: ucan_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 7/8] can: usb_8dev: usb_8dev_disconnect(): " Vincent Mailhol
2022-12-03 13:31 ` [PATCH 8/8] can: etas_es58x and peak_usb: remove useless call to usb_set_intfdata() Vincent Mailhol
2022-12-05 8:35 ` Oliver Neukum [this message]
2022-12-08 9:00 ` [PATCH 0/8] can: usb: remove all usb_set_intfdata(intf, NULL) in drivers' disconnect() Vincent MAILHOL
2022-12-08 10:55 ` Oliver Neukum
2022-12-08 15:44 ` Vincent MAILHOL
2022-12-08 16:28 ` Alan Stern
2022-12-08 16:51 ` Oliver Neukum
2022-12-10 9:02 ` Vincent MAILHOL
2022-12-10 9:01 ` [PATCH v2 0/9] " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 1/9] can: ems_usb: ems_usb_disconnect(): fix NULL pointer dereference Vincent Mailhol
2022-12-10 10:59 ` Johan Hovold
2022-12-11 11:24 ` Vincent MAILHOL
2022-12-10 9:01 ` [PATCH v2 2/9] can: esd_usb: esd_usb_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 3/9] can: gs_usb: gs_usb_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 4/9] can: kvaser_usb: kvaser_usb_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 5/9] can: mcba_usb: mcba_usb_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 6/9] can: ucan: ucan_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 7/9] can: usb_8dev: usb_8dev_disconnect(): " Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 8/9] can: usb: remove useless check on driver data Vincent Mailhol
2022-12-10 9:01 ` [PATCH v2 9/9] can: etas_es58x and peak_usb: remove useless call to usb_set_intfdata() Vincent Mailhol
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9493232b-c8fa-5612-fb13-fccf58b01942@suse.com \
--to=oneukum@suse.com \
--cc=Julia.Lawall@inria.fr \
--cc=anssi.hannula@bitwise.fi \
--cc=b.krumboeck@gmail.com \
--cc=cmoehring@christ-es.de \
--cc=davem@davemloft.net \
--cc=db@kvaser.com \
--cc=dzm91@hust.edu.cn \
--cc=edumazet@google.com \
--cc=extja@kvaser.com \
--cc=frank.jungclaus@esd.eu \
--cc=git@jbrengineering.co.uk \
--cc=gustavoars@kernel.org \
--cc=haas@ems-wuensche.com \
--cc=hbh25y@gmail.com \
--cc=jakob.unterwurzacher@theobroma-systems.com \
--cc=jhofstee@victronenergy.com \
--cc=kuba@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mailhol.vincent@wanadoo.fr \
--cc=martin.elshuber@theobroma-systems.com \
--cc=max@schneidersoft.net \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=olivier@sobrie.be \
--cc=pabeni@redhat.com \
--cc=paskripkin@gmail.com \
--cc=pfink@christ-es.de \
--cc=philipp.tomsich@theobroma-systems.com \
--cc=remigiusz.kollataj@mobica.com \
--cc=s.grosjean@peak-system.com \
--cc=socketcan@esd.eu \
--cc=socketcan@hartkopp.net \
--cc=stefan.maetje@esd.eu \
--cc=stern@rowland.harvard.edu \
--cc=vasanth.sadhasivan@samsara.com \
--cc=wg@grandegger.com \
--cc=wsa+renesas@sang-engineering.com \
--cc=yashi@spacecubics.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox