From: Johan Hovold <johan@kernel.org>
To: Andreas Kemnade <andreas@kemnade.info>
Cc: davem@davemloft.net, joe@perches.com, gregkh@linuxfoundation.org,
peter@hurleysoftware.com, hns@goldelico.com,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: hso: register netdev later to avoid a race condition
Date: Thu, 27 Apr 2017 10:44:01 +0200 [thread overview]
Message-ID: <20170427084401.GP2823@localhost> (raw)
In-Reply-To: <1493227600-10102-1-git-send-email-andreas@kemnade.info>
On Wed, Apr 26, 2017 at 07:26:40PM +0200, Andreas Kemnade wrote:
> If the netdev is accessed before the urbs are initialized,
> there will be NULL pointer dereferences. That is avoided by
> registering it when it is fully initialized.
> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> drivers/net/usb/hso.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 93411a3..00067a0 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2534,13 +2534,6 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
> SET_NETDEV_DEV(net, &interface->dev);
> SET_NETDEV_DEVTYPE(net, &hso_type);
>
> - /* registering our net device */
> - result = register_netdev(net);
> - if (result) {
> - dev_err(&interface->dev, "Failed to register device\n");
> - goto exit;
> - }
> -
> /* start allocating */
> for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
> hso_net->mux_bulk_rx_urb_pool[i] = usb_alloc_urb(0, GFP_KERNEL);
> @@ -2560,6 +2553,13 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
>
> add_net_device(hso_dev);
>
> + /* registering our net device */
> + result = register_netdev(net);
> + if (result) {
> + dev_err(&interface->dev, "Failed to register device\n");
> + goto exit;
This all looks good, but you should consider cleaning up the error
handling of this function as a follow-up as we should not be
deregistering netdevs that have never been registered (e.g. if a
required endpoint is missing or if registration fails for some reason).
But just to be clear, this problem existed also before this change.
> + }
> +
> hso_log_port(hso_dev);
>
> hso_create_rfkill(hso_dev, interface);
Reviewed-by: Johan Hovold <johan@kernel.org>
Thanks,
Johan
next prev parent reply other threads:[~2017-04-27 8:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-26 17:26 [PATCH] net: hso: register netdev later to avoid a race condition Andreas Kemnade
2017-04-27 8:44 ` Johan Hovold [this message]
2017-04-28 17:36 ` Andreas Kemnade
2017-04-28 19:00 ` Johan Hovold
2017-04-28 20:12 ` David Miller
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=20170427084401.GP2823@localhost \
--to=johan@kernel.org \
--cc=andreas@kemnade.info \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=hns@goldelico.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=peter@hurleysoftware.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;
as well as URLs for NNTP newsgroup(s).