| Message ID | 20210906124339.22264-1-johan@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | USB: cdc-acm: fix minor-number release | expand |
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
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;
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: stable@vger.kernel.org # 4.19 Cc: Jaejoong Kim <climbbb.kim@gmail.com> Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/class/cdc-acm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)