Re: Doubts about Patch "ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe"

[lyl2019]

Hello Dongliang Mu,

>  Depending on if CONFIG_PCI defines, the "tpci200->info->cfg_regs" may
> not be freed.
> 
> #ifdef CONFIG_PCI
> /* Destroy a virtual mapping cookie for a PCI BAR (memory or IO) */
> struct pci_dev;
> extern void pci_iounmap(struct pci_dev *dev, void __iomem *);
> #elif defined(CONFIG_GENERIC_IOMAP)
> struct pci_dev;
> static inline void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
> { }
> #endif

I think only `CONFIG_PCI=n` and `CONFIG_GENERIC_IOMAP=y` cause pci_iounmap an empty
implementation. Actually, `CONFIG_PCI` is a default option when run `make defconfig`,
pci_iounmap() usually is acted as an extern function.


> Even if CONFIG_PCI is undefined, it is possible that
> tpci200->info->cfg_regs is not freed at all. Therefore, this patch
> would cause memory leak. Take a look at the following code:
> 
> void pci_iounmap(struct pci_dev *dev, void __iomem * addr)
> {
>         IO_COND(addr, /* nothing */, iounmap(addr));
> }

Here i am not sure this is the final implementation of pci_iounmap(),
because pci_iounmap() is re-implementated in many architectures.
Even so, i observed there still many call-sites of pci_iounmap() have reset
`the addr = NULL` after calling.
Can you have some ways to determine the actual implementation of 
pci_iounmap in our cases?


> #define IO_COND(addr, is_pio, is_mmio) do {                     \
>         unsigned long port = (unsigned long __force)addr;       \
>         if (port >= PIO_RESERVED) {                             \
>                 is_mmio;                                        \
>         } else if (port > PIO_OFFSET) {                         \
>                 port &= PIO_MASK;                               \
>                 is_pio;                                         \
>         } else                                                  \
>                 bad_io_access(port, #is_pio );                  \
> } while (0)
> 

Although the above codes is actually called, the addr might be freed 
if `port >= PIO_RESERVED` is true. The double free still existed.



If I make any mistakes, please tell me.
Thanks your report.
---
Lv Yunlong