Le 26/10/2022 à 13:12, Tharunkumar.Pasumarthi@xxxxxxxxxxxxx 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
