usb: cdc-acm: Decrement tty port's refcount if probe() fail

usb: cdc-acm: Decrement tty port's refcount if probe() fail

[Jaejoong Kim]

The cdc-acm driver does not have a refcount of itself, but uses a
tty_port's refcount. That is, if the refcount of tty_port is '0', we
can clean up the cdc-acm driver by calling the .destruct()
callback function of struct tty_port_operations.

The problem is the destruct() callback function is not called if
the probe() fails, because tty_port's refcount is not zero. So,
add tty_port_put() when probe() fails.

Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
 drivers/usb/class/cdc-acm.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 8e0636c..e8d4cb7 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1375,6 +1375,9 @@ static int acm_probe(struct usb_interface *intf,
 	if (acm == NULL)
 		goto alloc_fail;
 
+	tty_port_init(&acm->port);
+	acm->port.ops = &acm_port_ops;
+
 	minor = acm_alloc_minor(acm);
 	if (minor < 0)
 		goto alloc_fail1;
@@ -1410,22 +1413,20 @@ static int acm_probe(struct usb_interface *intf,
 		acm->out = usb_sndintpipe(usb_dev, epwrite->bEndpointAddress);
 	else
 		acm->out = usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress);
-	tty_port_init(&acm->port);
-	acm->port.ops = &acm_port_ops;
 	init_usb_anchor(&acm->delayed);
 	acm->quirks = quirks;
 
 	buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma);
 	if (!buf)
-		goto alloc_fail2;
+		goto alloc_fail1;
 	acm->ctrl_buffer = buf;
 
 	if (acm_write_buffers_alloc(acm) < 0)
-		goto alloc_fail4;
+		goto alloc_fail2;
 
 	acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!acm->ctrlurb)
-		goto alloc_fail5;
+		goto alloc_fail3;
 
 	for (i = 0; i < num_rx_buf; i++) {
 		struct acm_rb *rb = &(acm->read_buffers[i]);
@@ -1434,13 +1435,13 @@ static int acm_probe(struct usb_interface *intf,
 		rb->base = usb_alloc_coherent(acm->dev, readsize, GFP_KERNEL,
 								&rb->dma);
 		if (!rb->base)
-			goto alloc_fail6;
+			goto alloc_fail4;
 		rb->index = i;
 		rb->instance = acm;
 
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!urb)
-			goto alloc_fail6;
+			goto alloc_fail4;
 
 		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 		urb->transfer_dma = rb->dma;
@@ -1462,7 +1463,7 @@ static int acm_probe(struct usb_interface *intf,
 
 		snd->urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (snd->urb == NULL)
-			goto alloc_fail7;
+			goto alloc_fail5;
 
 		if (usb_endpoint_xfer_int(epwrite))
 			usb_fill_int_urb(snd->urb, usb_dev, acm->out,
@@ -1480,7 +1481,7 @@ static int acm_probe(struct usb_interface *intf,
 
 	i = device_create_file(&intf->dev, &dev_attr_bmCapabilities);
 	if (i < 0)
-		goto alloc_fail7;
+		goto alloc_fail5;
 
 	if (h.usb_cdc_country_functional_desc) { /* export the country data */
 		struct usb_cdc_country_functional_desc * cfd =
@@ -1539,7 +1540,7 @@ static int acm_probe(struct usb_interface *intf,
 			&control_interface->dev);
 	if (IS_ERR(tty_dev)) {
 		rv = PTR_ERR(tty_dev);
-		goto alloc_fail8;
+		goto alloc_fail6;
 	}
 
 	if (quirks & CLEAR_HALT_CONDITIONS) {
@@ -1548,7 +1549,7 @@ static int acm_probe(struct usb_interface *intf,
 	}
 
 	return 0;
-alloc_fail8:
+alloc_fail6:
 	if (acm->country_codes) {
 		device_remove_file(&acm->control->dev,
 				&dev_attr_wCountryCodes);
@@ -1557,23 +1558,21 @@ static int acm_probe(struct usb_interface *intf,
 		kfree(acm->country_codes);
 	}
 	device_remove_file(&acm->control->dev, &dev_attr_bmCapabilities);
-alloc_fail7:
+alloc_fail5:
 	usb_set_intfdata(intf, NULL);
 	for (i = 0; i < ACM_NW; i++)
 		usb_free_urb(acm->wb[i].urb);
-alloc_fail6:
+alloc_fail4:
 	for (i = 0; i < num_rx_buf; i++)
 		usb_free_urb(acm->read_urbs[i]);
 	acm_read_buffers_free(acm);
 	usb_free_urb(acm->ctrlurb);
-alloc_fail5:
+alloc_fail3:
 	acm_write_buffers_free(acm);
-alloc_fail4:
-	usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
 alloc_fail2:
-	acm_release_minor(acm);
+	usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
 alloc_fail1:
-	kfree(acm);
+	tty_port_put(&acm->port);
 alloc_fail:
 	return rv;
 }

usb: cdc-acm: Decrement tty port's refcount if probe() fail

[Oliver Neukum]

On Do, 2018-06-21 at 17:45 +0900, Jaejoong Kim wrote:
> The cdc-acm driver does not have a refcount of itself, but uses a
> tty_port's refcount. That is, if the refcount of tty_port is '0', we
> can clean up the cdc-acm driver by calling the .destruct()
> callback function of struct tty_port_operations.
> 
> The problem is the destruct() callback function is not called if
> the probe() fails, because tty_port's refcount is not zero. So,
> add tty_port_put() when probe() fails.
> 
> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
Acked-by: Oliver Neukum <oneukum@suse.com>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usb: cdc-acm: Decrement tty port's refcount if probe() fail

[Oliver Neukum]

On Do, 2018-06-21 at 17:45 +0900, Jaejoong Kim wrote:
> The cdc-acm driver does not have a refcount of itself, but uses a
> tty_port's refcount. That is, if the refcount of tty_port is '0', we
> can clean up the cdc-acm driver by calling the .destruct()
> callback function of struct tty_port_operations.
> 
> The problem is the destruct() callback function is not called if
> the probe() fails, because tty_port's refcount is not zero. So,
> add tty_port_put() when probe() fails.
> 
> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>

Acked-by: Oliver Neukum <oneukum@suse.com>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html