Messages in this thread Patch in this message | | | Subject | Re: [PATCH] tty: nozomi: Fix a resource leak in an error handling function | From | Jiri Slaby <> | Date | Mon, 10 May 2021 11:51:16 +0200 |
| |
On 09. 05. 21, 19:22, Christophe JAILLET wrote: > A 'request_irq()' call is not balanced by a corresponding 'free_irq()' in > the error handling path, as already done in the remove function. > > Add it. > > Fixes: 9842c38e9176 ("kfifo: fix warn_unused_result") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
> --- > I also wonder if the loop above is correct. The 'i < MAX_PORT' looks really > spurious to me. > 'tty_port_destroy' can be called twice for the same entry (once before > branching in the error handling path, and once in here) and > 'tty_unregister_device'/'tty_port_destroy' will be called on entries > that have not been 'tty_port_init'ed or 'tty_port_register_device'd. > I don't know if it may be an issue.
Yes. The fail path handling is very broken there. Both the code of err_free_tty label, and of the err_free_kfifo label. The loops should have been _something_ (I didn't invest much thinking into it, so it's likely wrong) like: for (i--; i--; ) { ... }
> --- > drivers/tty/nozomi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c > index 9a2d78ace49b..b270e137ef9b 100644 > --- a/drivers/tty/nozomi.c > +++ b/drivers/tty/nozomi.c > @@ -1420,6 +1420,7 @@ static int nozomi_card_init(struct pci_dev *pdev, > tty_unregister_device(ntty_driver, dc->index_start + i); > tty_port_destroy(&dc->port[i].port); > } > + free_irq(pdev->irq, dc); > err_free_kfifo: > for (i = 0; i < MAX_PORT; i++) > kfifo_free(&dc->port[i].fifo_ul); >
thanks, -- js suse labs
| |