If the driver runs out of minor numbers it would release minor 0 and
allow another device to claim the minor while still in use.
Fortunately, registering the tty class device of the second device would
fail (with a stack dump) due to the sysfs name collision so no memory is
leaked.
Fixes: cae2bc768d17 ("usb: cdc-acm: Decrement tty port's refcount if probe() fail")
Cc: [email protected] # 4.19
Cc: Jaejoong Kim <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/class/cdc-acm.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 4895325b16a4..5f0260bc4469 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -726,7 +726,8 @@ static void acm_port_destruct(struct tty_port *port)
{
struct acm *acm = container_of(port, struct acm, port);
- acm_release_minor(acm);
+ if (acm->minor != ACM_TTY_MINORS)
+ acm_release_minor(acm);
usb_put_intf(acm->control);
kfree(acm->country_codes);
kfree(acm);
@@ -1323,8 +1324,10 @@ static int acm_probe(struct usb_interface *intf,
usb_get_intf(acm->control); /* undone in destruct() */
minor = acm_alloc_minor(acm);
- if (minor < 0)
+ if (minor < 0) {
+ acm->minor = ACM_TTY_MINORS;
goto err_put_port;
+ }
acm->minor = minor;
acm->dev = usb_dev;
--
2.32.0
On 06.09.21 14:43, Johan Hovold wrote:
> @@ -1323,8 +1324,10 @@ static int acm_probe(struct usb_interface *intf,
> usb_get_intf(acm->control); /* undone in destruct() */
>
> minor = acm_alloc_minor(acm);
> - if (minor < 0)
> + if (minor < 0) {
> + acm->minor = ACM_TTY_MINORS;
> goto err_put_port;
Hi,
Congratulations for catching that one.
May I request to improve understandability of the code that you give
the constant a distinct name for this purpose? Something like
ACM_MINOR_POISON or ACM_INVALID_MINOR
so that normal people can understand the fixed code?
Regards
Oliver
On Mon, Sep 06, 2021 at 03:02:43PM +0200, Oliver Neukum wrote:
>
> On 06.09.21 14:43, Johan Hovold wrote:
> > @@ -1323,8 +1324,10 @@ static int acm_probe(struct usb_interface *intf,
> > usb_get_intf(acm->control); /* undone in destruct() */
> >
> > minor = acm_alloc_minor(acm);
> > - if (minor < 0)
> > + if (minor < 0) {
> > + acm->minor = ACM_TTY_MINORS;
> > goto err_put_port;
> Congratulations for catching that one.
Heh, thanks.
> May I request to improve understandability of the code that you give
> the constant a distinct name for this purpose? Something like
>
> ACM_MINOR_POISON or ACM_INVALID_MINOR
>
> so that normal people can understand the fixed code?
Sure, I'll use ACM_MINOR_INVALID.
Johan