| Message ID | 1421756978-4093-5-git-send-email-olivier@sobrie.be |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote: > When the rfkill interface was created, a buffer containing the name > of the rfkill node was allocated. This buffer was never freed when the > device disappears. > > To fix the problem, we put the name given to rfkill_alloc() in > the hso_net structure. > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > --- > drivers/net/usb/hso.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c > index 470ef9e..a49ac2e 100644 > --- a/drivers/net/usb/hso.c > +++ b/drivers/net/usb/hso.c > @@ -153,6 +153,7 @@ struct hso_net { > struct hso_device *parent; > struct net_device *net; > struct rfkill *rfkill; > + char name[8]; > > struct usb_endpoint_descriptor *in_endp; > struct usb_endpoint_descriptor *out_endp; > @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev, > { > struct hso_net *hso_net = dev2net(hso_dev); > struct device *dev = &hso_net->net->dev; > - char *rfkn; > > - rfkn = kzalloc(20, GFP_KERNEL); > - if (!rfkn) > - dev_err(dev, "%s - Out of memory\n", __func__); > - > - snprintf(rfkn, 20, "hso-%d", > + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d", > interface->altsetting->desc.bInterfaceNumber); That number is not unique. Indeed it will be identical for all devices. Regards Oliver
On Tue, Jan 20, 2015 at 02:13:17PM +0100, Oliver Neukum wrote: > On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote: > > When the rfkill interface was created, a buffer containing the name > > of the rfkill node was allocated. This buffer was never freed when the > > device disappears. > > > > To fix the problem, we put the name given to rfkill_alloc() in > > the hso_net structure. > > > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > > --- > > drivers/net/usb/hso.c | 12 +++--------- > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c > > index 470ef9e..a49ac2e 100644 > > --- a/drivers/net/usb/hso.c > > +++ b/drivers/net/usb/hso.c > > @@ -153,6 +153,7 @@ struct hso_net { > > struct hso_device *parent; > > struct net_device *net; > > struct rfkill *rfkill; > > + char name[8]; > > > > struct usb_endpoint_descriptor *in_endp; > > struct usb_endpoint_descriptor *out_endp; > > @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev, > > { > > struct hso_net *hso_net = dev2net(hso_dev); > > struct device *dev = &hso_net->net->dev; > > - char *rfkn; > > > > - rfkn = kzalloc(20, GFP_KERNEL); > > - if (!rfkn) > > - dev_err(dev, "%s - Out of memory\n", __func__); > > - > > - snprintf(rfkn, 20, "hso-%d", > > + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d", > > interface->altsetting->desc.bInterfaceNumber); > > That number is not unique. Indeed it will be identical for all devices. Indeed. That should be corrected too. Thank you, Olivier -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-01-20 at 14:13 +0100, Oliver Neukum wrote: > On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote: > > When the rfkill interface was created, a buffer containing the name > > of the rfkill node was allocated. This buffer was never freed when the > > device disappears. > > > > To fix the problem, we put the name given to rfkill_alloc() in > > the hso_net structure. > > > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > > --- > > drivers/net/usb/hso.c | 12 +++--------- > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c > > index 470ef9e..a49ac2e 100644 > > --- a/drivers/net/usb/hso.c > > +++ b/drivers/net/usb/hso.c > > @@ -153,6 +153,7 @@ struct hso_net { > > struct hso_device *parent; > > struct net_device *net; > > struct rfkill *rfkill; > > + char name[8]; > > > > struct usb_endpoint_descriptor *in_endp; > > struct usb_endpoint_descriptor *out_endp; > > @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev, > > { > > struct hso_net *hso_net = dev2net(hso_dev); > > struct device *dev = &hso_net->net->dev; > > - char *rfkn; > > > > - rfkn = kzalloc(20, GFP_KERNEL); > > - if (!rfkn) > > - dev_err(dev, "%s - Out of memory\n", __func__); > > - > > - snprintf(rfkn, 20, "hso-%d", > > + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d", > > interface->altsetting->desc.bInterfaceNumber); > > That number is not unique. Indeed it will be identical for all devices. I would say just do "static u32 rfkill_counter = 0" and + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d", + rfkill_counter++); We can't just use the netdev's name because that may have conflicts. eg, the netdev will get hso0 when plugged in (and thus rfkill would get hso-0) but then udev will rename that to something like wwp0s26f7u2i8. Then the second HSO you plug in will get the name 'hso0', and so the second rfkill would get 'hso-0', but that's already taken by the first rfkill... Which is why I just suggest a counter. Dan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 470ef9e..a49ac2e 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -153,6 +153,7 @@ struct hso_net { struct hso_device *parent; struct net_device *net; struct rfkill *rfkill; + char name[8]; struct usb_endpoint_descriptor *in_endp; struct usb_endpoint_descriptor *out_endp; @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev, { struct hso_net *hso_net = dev2net(hso_dev); struct device *dev = &hso_net->net->dev; - char *rfkn; - rfkn = kzalloc(20, GFP_KERNEL); - if (!rfkn) - dev_err(dev, "%s - Out of memory\n", __func__); - - snprintf(rfkn, 20, "hso-%d", + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d", interface->altsetting->desc.bInterfaceNumber); - hso_net->rfkill = rfkill_alloc(rfkn, + hso_net->rfkill = rfkill_alloc(hso_net->name, &interface_to_usbdev(interface)->dev, RFKILL_TYPE_WWAN, &hso_rfkill_ops, hso_dev); if (!hso_net->rfkill) { dev_err(dev, "%s - Out of memory\n", __func__); - kfree(rfkn); return; } if (rfkill_register(hso_net->rfkill) < 0) { rfkill_destroy(hso_net->rfkill); - kfree(rfkn); hso_net->rfkill = NULL; dev_err(dev, "%s - Failed to register rfkill\n", __func__); return;
When the rfkill interface was created, a buffer containing the name of the rfkill node was allocated. This buffer was never freed when the device disappears. To fix the problem, we put the name given to rfkill_alloc() in the hso_net structure. Signed-off-by: Olivier Sobrie <olivier@sobrie.be> --- drivers/net/usb/hso.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)