Re: [PATCH v2 tty-next 1/3] 8250: microchip: pci1xxxx: Add driver for quad-uart support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux