[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <61826266-3f02-bc04-bd98-6391fe5d9fa6@wanadoo.fr>
Date: Wed, 26 Oct 2022 22:10:40 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Tharunkumar.Pasumarthi@...rochip.com, jk@...abs.org,
Kumaravel.Thiagarajan@...rochip.com, gregkh@...uxfoundation.org,
ilpo.jarvinen@...ux.intel.com, wander@...hat.com,
u.kleine-koenig@...gutronix.de, etremblay@...tech-controls.com,
macro@...am.me.uk, jirislaby@...nel.org, johan@...nel.org,
andy.shevchenko@...il.com, geert+renesas@...der.be,
phil.edworthy@...esas.com, lukas@...ner.de
Cc: linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v2 tty-next 1/3] 8250: microchip: pci1xxxx: Add driver for
quad-uart support.
Le 26/10/2022 à 13:12, Tharunkumar.Pasumarthi@...rochip.com a écrit :
> On Mon, 2022-10-03 at 21:36 +0200, Christophe JAILLET wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>> + }
>>> + priv->line[i] = serial8250_register_8250_port(&uart);
>>
>> In case of error, this should be undone in an error handling path in the
>> probe, as done in the remove() function below.
>>
>> If we break, we still continue and return success. But the last
>> priv->line[i] are still 0. Is it an issue when pci1xxxx_serial_remove()
>> is called?
>
> This will not cause a problem in pci1xxxx_serial_remove as this condition 'priv-
>> line[i] >= 0' will be checked for each of the ports before calling
> serial8250_unregister_port API.
Yes, this is my point.
We check for >=0 in pci1xxxx_serial_remove(), but if we have an error in
the "for (i = 0; i < nr_ports; i++)" loop, some line[i] we'll still be
zeroed because 'priv' is zalloc'ed.
In such a case, the probe still succeeds.
So, if pci1xxxx_serial_remove() is called later, we potentially call
serial8250_unregister_port(0) several times.
This can lead to double (or more) free in serial8250_em485_destroy()
(but maybe this path can't be taken here?) or maybe some other troubles
elsewhere (I've not checked all the logic in uart_remove_one_port() to
make sure that calling several times this function with the same args is
fine).
So my point is maybe just a "can't happen" case.
If so, apologize for the noise.
CJ
>
>
> Thanks,
> Tharun Kumar P
Powered by blists - more mailing lists