On Thu, 27 Apr 2017 10:44:01 +0200 Johan Hovold <jo...@kernel.org> wrote:
> 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 <h...@goldelico.com>
> > Signed-off-by: Andreas Kemnade <andr...@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.
>
Just to check wether I am understanding this correctly. In your opinion
this patch is good for now. And later when it is applied, there should
be an additional error handling cleanup patch.
Regards,
Andreas
pgpFfKVY2MFRl.pgp
Description: OpenPGP digital signature
