[PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Dongliang Mu]

The function tpci200_register called by tpci200_install and
tpci200_unregister called by tpci200_uninstall are in pair. However,
tpci200_unregister has some cleanup operations not in the
tpci200_register. So the error handling code of tpci200_pci_probe has
many different double free issues.

Fix this problem by moving those cleanup operations out of
tpci200_unregister, into tpci200_pci_remove and reverting
the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
Fix a double free in tpci200_pci_probe").

Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
location change of tpci_unregister into one separate patch;
 drivers/ipack/carriers/tpci200.c | 35 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index 3461b0a7dc62..3f198b2405e3 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -89,16 +89,13 @@ static void tpci200_unregister(struct tpci200_board *tpci200)
 	free_irq(tpci200->info->pdev->irq, (void *) tpci200);
 
 	pci_iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
-	pci_iounmap(tpci200->info->pdev, tpci200->info->cfg_regs);
 
 	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
 	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
 	pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
 	pci_release_region(tpci200->info->pdev, TPCI200_MEM8_SPACE_BAR);
-	pci_release_region(tpci200->info->pdev, TPCI200_CFG_MEM_BAR);
 
 	pci_disable_device(tpci200->info->pdev);
-	pci_dev_put(tpci200->info->pdev);
 }
 
 static void tpci200_enable_irq(struct tpci200_board *tpci200,
@@ -527,7 +524,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 	tpci200->info = kzalloc(sizeof(struct tpci200_infos), GFP_KERNEL);
 	if (!tpci200->info) {
 		ret = -ENOMEM;
-		goto out_err_info;
+		goto err_tpci200;
 	}
 
 	pci_dev_get(pdev);
@@ -538,7 +535,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to allocate PCI Configuration Memory");
 		ret = -EBUSY;
-		goto out_err_pci_request;
+		goto err_tpci200_info;
 	}
 	tpci200->info->cfg_regs = ioremap(
 			pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
@@ -546,7 +543,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 	if (!tpci200->info->cfg_regs) {
 		dev_err(&pdev->dev, "Failed to map PCI Configuration Memory");
 		ret = -EFAULT;
-		goto out_err_ioremap;
+		goto err_request_region;
 	}
 
 	/* Disable byte swapping for 16 bit IP module access. This will ensure
@@ -569,7 +566,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 	if (ret) {
 		dev_err(&pdev->dev, "error during tpci200 install\n");
 		ret = -ENODEV;
-		goto out_err_install;
+		goto err_cfg_regs;
 	}
 
 	/* Register the carrier in the industry pack bus driver */
@@ -581,7 +578,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 		dev_err(&pdev->dev,
 			"error registering the carrier on ipack driver\n");
 		ret = -EFAULT;
-		goto out_err_bus_register;
+		goto err_tpci200_install;
 	}
 
 	/* save the bus number given by ipack to logging purpose */
@@ -592,19 +589,16 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 		tpci200_create_device(tpci200, i);
 	return 0;
 
-out_err_bus_register:
+err_tpci200_install:
 	tpci200_uninstall(tpci200);
-	/* tpci200->info->cfg_regs is unmapped in tpci200_uninstall */
-	tpci200->info->cfg_regs = NULL;
-out_err_install:
-	if (tpci200->info->cfg_regs)
-		iounmap(tpci200->info->cfg_regs);
-out_err_ioremap:
+err_cfg_regs:
+	pci_iounmap(tpci200->info->cfg_regs);
+err_request_region:
 	pci_release_region(pdev, TPCI200_CFG_MEM_BAR);
-out_err_pci_request:
-	pci_dev_put(pdev);
+err_tpci200_info:
 	kfree(tpci200->info);
-out_err_info:
+	pci_dev_put(pdev);
+err_tpci200:
 	kfree(tpci200);
 	return ret;
 }
@@ -614,6 +608,11 @@ static void __tpci200_pci_remove(struct tpci200_board *tpci200)
 	ipack_bus_unregister(tpci200->info->ipack_bus);
 	tpci200_uninstall(tpci200);
 
+	pci_iounmap(tpci200->info->cfg_regs);
+	pci_release_region(tpci200->info->pdev, TPCI200_CFG_MEM_BAR);
+
+	pci_dev_put(tpci200->info->pdev);
+
 	kfree(tpci200->info);
 	kfree(tpci200);
 }
-- 
2.25.1

[PATCH v2 2/4] ipack: tpci200: fix memory leak in the tpci200_register

[Dongliang Mu]

The error handling code in tpci200_register does not free interface_regs
allocated by ioremap and the current version of error handling code is
problematic.

Fix this by refactoring the error handling code and free interface_regs
when necessary.

Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Fixes: 43986798fd50 ("ipack: add error handling for ioremap_nocache")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
 drivers/ipack/carriers/tpci200.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index 3f198b2405e3..b1562b881cd1 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -254,7 +254,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			"(bn 0x%X, sn 0x%X) failed to allocate PCI resource for BAR 2 !",
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
-		goto out_disable_pci;
+		goto err_disable_device;
 	}
 
 	/* Request IO ID INT space (Bar 3) */
@@ -266,7 +266,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			"(bn 0x%X, sn 0x%X) failed to allocate PCI resource for BAR 3 !",
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
-		goto out_release_ip_space;
+		goto err_ip_interface_bar;
 	}
 
 	/* Request MEM8 space (Bar 5) */
@@ -277,7 +277,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			"(bn 0x%X, sn 0x%X) failed to allocate PCI resource for BAR 5!",
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
-		goto out_release_ioid_int_space;
+		goto err_io_id_int_spaces_bar;
 	}
 
 	/* Request MEM16 space (Bar 4) */
@@ -288,7 +288,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			"(bn 0x%X, sn 0x%X) failed to allocate PCI resource for BAR 4!",
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
-		goto out_release_mem8_space;
+		goto err_mem8_space_bar;
 	}
 
 	/* Map internal tpci200 driver user space */
@@ -302,7 +302,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
 		res = -ENOMEM;
-		goto out_release_mem8_space;
+		goto err_mem16_space_bar;
 	}
 
 	/* Initialize lock that protects interface_regs */
@@ -341,18 +341,22 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			"(bn 0x%X, sn 0x%X) unable to register IRQ !",
 			tpci200->info->pdev->bus->number,
 			tpci200->info->pdev->devfn);
-		goto out_release_ioid_int_space;
+		goto err_interface_regs;
 	}
 
 	return 0;
 
-out_release_mem8_space:
+err_interface_regs:
+	pci_iounmap(tpci200->info->interface_regs);
+err_mem16_space_bar:
+	pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
+err_mem8_space_bar:
 	pci_release_region(tpci200->info->pdev, TPCI200_MEM8_SPACE_BAR);
-out_release_ioid_int_space:
+err_io_id_int_spaces_bar:
 	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
-out_release_ip_space:
+err_ip_interface_bar:
 	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
-out_disable_pci:
+err_disable_device:
 	pci_disable_device(tpci200->info->pdev);
 	return res;
 }
-- 
2.25.1

Re: [PATCH v2 2/4] ipack: tpci200: fix memory leak in the tpci200_register

[Greg Kroah-Hartman]

On Mon, Aug 09, 2021 at 10:30:27PM +0800, Dongliang Mu wrote:
> The error handling code in tpci200_register does not free interface_regs
> allocated by ioremap and the current version of error handling code is
> problematic.
> 
> Fix this by refactoring the error handling code and free interface_regs
> when necessary.
> 
> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> Fixes: 43986798fd50 ("ipack: add error handling for ioremap_nocache")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>  drivers/ipack/carriers/tpci200.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

Also for stable?

Re: [PATCH v2 2/4] ipack: tpci200: fix memory leak in the tpci200_register

[Dongliang Mu]

On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 09, 2021 at 10:30:27PM +0800, Dongliang Mu wrote:
> > The error handling code in tpci200_register does not free interface_regs
> > allocated by ioremap and the current version of error handling code is
> > problematic.
> >
> > Fix this by refactoring the error handling code and free interface_regs
> > when necessary.
> >
> > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > Fixes: 43986798fd50 ("ipack: add error handling for ioremap_nocache")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> >  drivers/ipack/carriers/tpci200.c | 24 ++++++++++++++----------
> >  1 file changed, 14 insertions(+), 10 deletions(-)
>
> Also for stable?

Yes, it should.

Re: [PATCH v2 2/4] ipack: tpci200: fix memory leak in the tpci200_register

[Greg Kroah-Hartman]

On Tue, Aug 10, 2021 at 07:09:15AM +0800, Dongliang Mu wrote:
> On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Aug 09, 2021 at 10:30:27PM +0800, Dongliang Mu wrote:
> > > The error handling code in tpci200_register does not free interface_regs
> > > allocated by ioremap and the current version of error handling code is
> > > problematic.
> > >
> > > Fix this by refactoring the error handling code and free interface_regs
> > > when necessary.
> > >
> > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > Fixes: 43986798fd50 ("ipack: add error handling for ioremap_nocache")
> > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > ---
> > >  drivers/ipack/carriers/tpci200.c | 24 ++++++++++++++----------
> > >  1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > Also for stable?
> 
> Yes, it should.

Then please resend it, and the other commits that you wish to see go to
the stable tree, with the needed "cc: stable..." line in the
signed-off-by area.

thanks,

greg k-h

[PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[Dongliang Mu]

The deallocation api for ioremap should be iounmap, other than
pci_iounmap.

Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
 drivers/ipack/carriers/tpci200.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index b1562b881cd1..307f94f59c18 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -88,7 +88,7 @@ static void tpci200_unregister(struct tpci200_board *tpci200)
 {
 	free_irq(tpci200->info->pdev->irq, (void *) tpci200);
 
-	pci_iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
+	iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
 
 	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
 	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
@@ -347,7 +347,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 	return 0;
 
 err_interface_regs:
-	pci_iounmap(tpci200->info->interface_regs);
+	iounmap(tpci200->info->interface_regs);
 err_mem16_space_bar:
 	pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
 err_mem8_space_bar:
@@ -596,7 +596,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 err_tpci200_install:
 	tpci200_uninstall(tpci200);
 err_cfg_regs:
-	pci_iounmap(tpci200->info->cfg_regs);
+	iounmap(tpci200->info->cfg_regs);
 err_request_region:
 	pci_release_region(pdev, TPCI200_CFG_MEM_BAR);
 err_tpci200_info:
@@ -612,7 +612,7 @@ static void __tpci200_pci_remove(struct tpci200_board *tpci200)
 	ipack_bus_unregister(tpci200->info->ipack_bus);
 	tpci200_uninstall(tpci200);
 
-	pci_iounmap(tpci200->info->cfg_regs);
+	iounmap(tpci200->info->cfg_regs);
 	pci_release_region(tpci200->info->pdev, TPCI200_CFG_MEM_BAR);
 
 	pci_dev_put(tpci200->info->pdev);
-- 
2.25.1

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[Greg Kroah-Hartman]

On Mon, Aug 09, 2021 at 10:30:28PM +0800, Dongliang Mu wrote:
> The deallocation api for ioremap should be iounmap, other than
> pci_iounmap.
> 
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>  drivers/ipack/carriers/tpci200.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

This should just go for 5.15-rc1, right?  It should be a separate series
independant of the first 2?

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]

Hi Dongliang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc5 next-20210809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 36a21d51725af2ce0700c6ebcb6b9594aac658a6
config: i386-randconfig-a004-20210808 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/aab1c45ab3f37910ebb6f00f951c74ee88e25094
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
        git checkout aab1c45ab3f37910ebb6f00f951c74ee88e25094
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/ipack/carriers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/io.h:13,
                    from include/linux/pci.h:39,
                    from drivers/ipack/carriers/tpci200.h:14,
                    from drivers/ipack/carriers/tpci200.c:12:
   drivers/ipack/carriers/tpci200.c: In function 'tpci200_unregister':
>> arch/x86/include/asm/io.h:210:17: error: too many arguments to function 'iounmap'
     210 | #define iounmap iounmap
         |                 ^~~~~~~
   drivers/ipack/carriers/tpci200.c:91:2: note: in expansion of macro 'iounmap'
      91 |  iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
         |  ^~~~~~~
   arch/x86/include/asm/io.h:209:13: note: declared here
     209 | extern void iounmap(volatile void __iomem *addr);
         |             ^~~~~~~


vim +/iounmap +210 arch/x86/include/asm/io.h

133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  208  
133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  209  extern void iounmap(volatile void __iomem *addr);
80b9ece1339374 Andy Shevchenko     2017-06-30 @210  #define iounmap iounmap
133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  211  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31270 bytes --]

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]

Hi Dongliang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc5 next-20210809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 36a21d51725af2ce0700c6ebcb6b9594aac658a6
config: i386-randconfig-a004-20210808 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/aab1c45ab3f37910ebb6f00f951c74ee88e25094
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
        git checkout aab1c45ab3f37910ebb6f00f951c74ee88e25094
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/ipack/carriers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/io.h:13,
                    from include/linux/pci.h:39,
                    from drivers/ipack/carriers/tpci200.h:14,
                    from drivers/ipack/carriers/tpci200.c:12:
   drivers/ipack/carriers/tpci200.c: In function 'tpci200_unregister':
>> arch/x86/include/asm/io.h:210:17: error: too many arguments to function 'iounmap'
     210 | #define iounmap iounmap
         |                 ^~~~~~~
   drivers/ipack/carriers/tpci200.c:91:2: note: in expansion of macro 'iounmap'
      91 |  iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
         |  ^~~~~~~
   arch/x86/include/asm/io.h:209:13: note: declared here
     209 | extern void iounmap(volatile void __iomem *addr);
         |             ^~~~~~~


vim +/iounmap +210 arch/x86/include/asm/io.h

133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  208  
133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  209  extern void iounmap(volatile void __iomem *addr);
80b9ece1339374 Andy Shevchenko     2017-06-30 @210  #define iounmap iounmap
133822c5c038b2 Jeremy Fitzhardinge 2009-02-06  211  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31270 bytes --]

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[Andy Shevchenko]

On Mon, Aug 9, 2021 at 5:42 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> The deallocation api for ioremap should be iounmap, other than

API
ioremap()
iounmap()

> pci_iounmap.

pci_iounmap()

Since you haven't compiled this, for the next version consider to
switch to devm_*() and pcim_*() APIs.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4177 bytes --]

Hi Dongliang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc5 next-20210809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 36a21d51725af2ce0700c6ebcb6b9594aac658a6
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/aab1c45ab3f37910ebb6f00f951c74ee88e25094
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
        git checkout aab1c45ab3f37910ebb6f00f951c74ee88e25094
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/ipack/carriers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/io.h:13,
                    from include/linux/irq.h:20,
                    from arch/ia64/include/asm/hardirq.h:19,
                    from include/linux/hardirq.h:11,
                    from include/linux/interrupt.h:11,
                    from include/linux/pci.h:38,
                    from drivers/ipack/carriers/tpci200.h:14,
                    from drivers/ipack/carriers/tpci200.c:12:
   drivers/ipack/carriers/tpci200.c: In function 'tpci200_unregister':
>> arch/ia64/include/asm/io.h:268:17: error: too many arguments to function 'iounmap'
     268 | #define iounmap iounmap
         |                 ^~~~~~~
   drivers/ipack/carriers/tpci200.c:91:2: note: in expansion of macro 'iounmap'
      91 |  iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
         |  ^~~~~~~
   arch/ia64/include/asm/io.h:260:13: note: declared here
     260 | extern void iounmap (volatile void __iomem *addr);
         |             ^~~~~~~


vim +/iounmap +268 arch/ia64/include/asm/io.h

ffc45571dfb4b7 include/asm-ia64/io.h      Aron Griffis      2006-10-17  257  
e9b0a0712148ab include/asm-ia64/io.h      Bjorn Helgaas     2006-03-26  258  extern void __iomem * ioremap(unsigned long offset, unsigned long size);
fded1829a24b34 arch/ia64/include/asm/io.h Christoph Hellwig 2019-08-11  259  extern void __iomem * ioremap_uc(unsigned long offset, unsigned long size);
9b50ffb0c0281b include/asm-ia64/io.h      Bjorn Helgaas     2007-03-30  260  extern void iounmap (volatile void __iomem *addr);
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  261  static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size)
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  262  {
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  263  	return ioremap(phys_addr, size);
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  264  }
0bbf47eab46975 arch/ia64/include/asm/io.h Arnd Bergmann     2018-07-25  265  #define ioremap ioremap
92281dee825f6d arch/ia64/include/asm/io.h Dan Williams      2015-08-10  266  #define ioremap_cache ioremap_cache
fded1829a24b34 arch/ia64/include/asm/io.h Christoph Hellwig 2019-08-11  267  #define ioremap_uc ioremap_uc
0bbf47eab46975 arch/ia64/include/asm/io.h Arnd Bergmann     2018-07-25 @268  #define iounmap iounmap
^1da177e4c3f41 include/asm-ia64/io.h      Linus Torvalds    2005-04-16  269  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64473 bytes --]

Re: [PATCH v2 3/4] ipack: tpci200: change pci_iounmap to iounmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]

Hi Dongliang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc5 next-20210809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 36a21d51725af2ce0700c6ebcb6b9594aac658a6
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/aab1c45ab3f37910ebb6f00f951c74ee88e25094
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongliang-Mu/ipack-tpci200-fix-many-double-free-issues-in-tpci200_pci_probe/20210809-223416
        git checkout aab1c45ab3f37910ebb6f00f951c74ee88e25094
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/ipack/carriers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/io.h:13,
                    from include/linux/irq.h:20,
                    from arch/ia64/include/asm/hardirq.h:19,
                    from include/linux/hardirq.h:11,
                    from include/linux/interrupt.h:11,
                    from include/linux/pci.h:38,
                    from drivers/ipack/carriers/tpci200.h:14,
                    from drivers/ipack/carriers/tpci200.c:12:
   drivers/ipack/carriers/tpci200.c: In function 'tpci200_unregister':
>> arch/ia64/include/asm/io.h:268:17: error: too many arguments to function 'iounmap'
     268 | #define iounmap iounmap
         |                 ^~~~~~~
   drivers/ipack/carriers/tpci200.c:91:2: note: in expansion of macro 'iounmap'
      91 |  iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
         |  ^~~~~~~
   arch/ia64/include/asm/io.h:260:13: note: declared here
     260 | extern void iounmap (volatile void __iomem *addr);
         |             ^~~~~~~


vim +/iounmap +268 arch/ia64/include/asm/io.h

ffc45571dfb4b7 include/asm-ia64/io.h      Aron Griffis      2006-10-17  257  
e9b0a0712148ab include/asm-ia64/io.h      Bjorn Helgaas     2006-03-26  258  extern void __iomem * ioremap(unsigned long offset, unsigned long size);
fded1829a24b34 arch/ia64/include/asm/io.h Christoph Hellwig 2019-08-11  259  extern void __iomem * ioremap_uc(unsigned long offset, unsigned long size);
9b50ffb0c0281b include/asm-ia64/io.h      Bjorn Helgaas     2007-03-30  260  extern void iounmap (volatile void __iomem *addr);
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  261  static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size)
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  262  {
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  263  	return ioremap(phys_addr, size);
6d5bbf00d251cc arch/ia64/include/asm/io.h Len Brown         2011-01-07  264  }
0bbf47eab46975 arch/ia64/include/asm/io.h Arnd Bergmann     2018-07-25  265  #define ioremap ioremap
92281dee825f6d arch/ia64/include/asm/io.h Dan Williams      2015-08-10  266  #define ioremap_cache ioremap_cache
fded1829a24b34 arch/ia64/include/asm/io.h Christoph Hellwig 2019-08-11  267  #define ioremap_uc ioremap_uc
0bbf47eab46975 arch/ia64/include/asm/io.h Arnd Bergmann     2018-07-25 @268  #define iounmap iounmap
^1da177e4c3f41 include/asm-ia64/io.h      Linus Torvalds    2005-04-16  269  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 64473 bytes --]

[PATCH v2 4/4] ipack: tpci200: move tpci200_unregister close to tpci200_register

[Dongliang Mu]

Move tpci200_unregister close to tpci200_register, then it is easier to
review the code related to the registration and unregistration

Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
 drivers/ipack/carriers/tpci200.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index 307f94f59c18..d553b4941539 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -84,20 +84,6 @@ static void tpci200_set_mask(struct tpci200_board *tpci200,
 	spin_unlock_irqrestore(&tpci200->regs_lock, flags);
 }
 
-static void tpci200_unregister(struct tpci200_board *tpci200)
-{
-	free_irq(tpci200->info->pdev->irq, (void *) tpci200);
-
-	iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
-
-	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
-	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
-	pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
-	pci_release_region(tpci200->info->pdev, TPCI200_MEM8_SPACE_BAR);
-
-	pci_disable_device(tpci200->info->pdev);
-}
-
 static void tpci200_enable_irq(struct tpci200_board *tpci200,
 			       int islot)
 {
@@ -236,6 +222,20 @@ static int tpci200_request_irq(struct ipack_device *dev,
 	return res;
 }
 
+static void tpci200_unregister(struct tpci200_board *tpci200)
+{
+	free_irq(tpci200->info->pdev->irq, (void *) tpci200);
+
+	iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
+
+	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
+	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
+	pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
+	pci_release_region(tpci200->info->pdev, TPCI200_MEM8_SPACE_BAR);
+
+	pci_disable_device(tpci200->info->pdev);
+}
+
 static int tpci200_register(struct tpci200_board *tpci200)
 {
 	int i;
-- 
2.25.1

Re: [PATCH v2 4/4] ipack: tpci200: move tpci200_unregister close to tpci200_register

[Greg Kroah-Hartman]

On Mon, Aug 09, 2021 at 10:30:29PM +0800, Dongliang Mu wrote:
> Move tpci200_unregister close to tpci200_register, then it is easier to
> review the code related to the registration and unregistration
> 
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>  drivers/ipack/carriers/tpci200.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

Again, independent of the first 2, and for 5.15-rc1, right?

Re: [PATCH v2 4/4] ipack: tpci200: move tpci200_unregister close to tpci200_register

[Dongliang Mu]

On Mon, Aug 9, 2021 at 11:33 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 09, 2021 at 10:30:29PM +0800, Dongliang Mu wrote:
> > Move tpci200_unregister close to tpci200_register, then it is easier to
> > review the code related to the registration and unregistration
> >
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> >  drivers/ipack/carriers/tpci200.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
>
> Again, independent of the first 2, and for 5.15-rc1, right?

No, those two cleanup PATCH 3/4 and 4/4 depends on first 2.

>

Re: [PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Greg Kroah-Hartman]

On Mon, Aug 09, 2021 at 10:30:26PM +0800, Dongliang Mu wrote:
> The function tpci200_register called by tpci200_install and
> tpci200_unregister called by tpci200_uninstall are in pair. However,
> tpci200_unregister has some cleanup operations not in the
> tpci200_register. So the error handling code of tpci200_pci_probe has
> many different double free issues.
> 
> Fix this problem by moving those cleanup operations out of
> tpci200_unregister, into tpci200_pci_remove and reverting
> the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
> Fix a double free in tpci200_pci_probe").
> 
> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
> location change of tpci_unregister into one separate patch;

Also needs to go to the stable trees, right?

Re: [PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Dongliang Mu]

On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 09, 2021 at 10:30:26PM +0800, Dongliang Mu wrote:
> > The function tpci200_register called by tpci200_install and
> > tpci200_unregister called by tpci200_uninstall are in pair. However,
> > tpci200_unregister has some cleanup operations not in the
> > tpci200_register. So the error handling code of tpci200_pci_probe has
> > many different double free issues.
> >
> > Fix this problem by moving those cleanup operations out of
> > tpci200_unregister, into tpci200_pci_remove and reverting
> > the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
> > Fix a double free in tpci200_pci_probe").
> >
> > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> > v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
> > location change of tpci_unregister into one separate patch;
>
> Also needs to go to the stable trees, right?

Yes, this needs to go to the stable trees.

>

Re: [PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Dongliang Mu]

On Tue, Aug 10, 2021 at 7:08 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Aug 09, 2021 at 10:30:26PM +0800, Dongliang Mu wrote:
> > > The function tpci200_register called by tpci200_install and
> > > tpci200_unregister called by tpci200_uninstall are in pair. However,
> > > tpci200_unregister has some cleanup operations not in the
> > > tpci200_register. So the error handling code of tpci200_pci_probe has
> > > many different double free issues.
> > >
> > > Fix this problem by moving those cleanup operations out of
> > > tpci200_unregister, into tpci200_pci_remove and reverting
> > > the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
> > > Fix a double free in tpci200_pci_probe").
> > >
> > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
> > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > ---
> > > v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
> > > location change of tpci_unregister into one separate patch;
> >
> > Also needs to go to the stable trees, right?
>
> Yes, this needs to go to the stable trees.

Hi gregkh,

Let me clarify more. In my series, PATCH 3/4 4/4 depends on PATCH 1/4
and PATCH 2/4. And also PATCH 2/4 depends on PATCH 1/4 as they are
closely related.

But from your reply, the last 2 patches should not depend on the first
2 patches. I don't quite understand as I don't send some patch series
before. For a patch series, the latter ones should depend on the
former ones, right? If I have any misunderstanding, please let me
know.

BTW, PATCH 3/4 has some compilation issues. I will fix it in the next version.


>
> >

Re: [PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Greg Kroah-Hartman]

On Tue, Aug 10, 2021 at 07:41:55AM +0800, Dongliang Mu wrote:
> On Tue, Aug 10, 2021 at 7:08 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Aug 09, 2021 at 10:30:26PM +0800, Dongliang Mu wrote:
> > > > The function tpci200_register called by tpci200_install and
> > > > tpci200_unregister called by tpci200_uninstall are in pair. However,
> > > > tpci200_unregister has some cleanup operations not in the
> > > > tpci200_register. So the error handling code of tpci200_pci_probe has
> > > > many different double free issues.
> > > >
> > > > Fix this problem by moving those cleanup operations out of
> > > > tpci200_unregister, into tpci200_pci_remove and reverting
> > > > the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
> > > > Fix a double free in tpci200_pci_probe").
> > > >
> > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
> > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > ---
> > > > v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
> > > > location change of tpci_unregister into one separate patch;
> > >
> > > Also needs to go to the stable trees, right?
> >
> > Yes, this needs to go to the stable trees.
> 
> Hi gregkh,
> 
> Let me clarify more. In my series, PATCH 3/4 4/4 depends on PATCH 1/4
> and PATCH 2/4. And also PATCH 2/4 depends on PATCH 1/4 as they are
> closely related.
> 
> But from your reply, the last 2 patches should not depend on the first
> 2 patches. I don't quite understand as I don't send some patch series
> before. For a patch series, the latter ones should depend on the
> former ones, right? If I have any misunderstanding, please let me
> know.

Yes, they can depend on previous patches, but if some patches are to be
merged today for 5.14-final, and others later for 5.15-rc1, then ideally
they will be separate series of changes as those go to two different
branches in my tree at the moment.

> BTW, PATCH 3/4 has some compilation issues. I will fix it in the next version.

As you haven't even tested these, I'm really hesitant to take them at
all.

Please just send the first two patches, fixed up, as a series after you
have tested them, and then after they are merged into Linus's tree, you
can send the cleanup patches, as they are just "nice" to have.

thanks,

greg k-h

Re: [PATCH v2 1/4] ipack: tpci200: fix many double free issues in tpci200_pci_probe

[Dongliang Mu]

On Tue, Aug 10, 2021 at 2:12 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 10, 2021 at 07:41:55AM +0800, Dongliang Mu wrote:
> > On Tue, Aug 10, 2021 at 7:08 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >
> > > On Mon, Aug 9, 2021 at 11:32 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Mon, Aug 09, 2021 at 10:30:26PM +0800, Dongliang Mu wrote:
> > > > > The function tpci200_register called by tpci200_install and
> > > > > tpci200_unregister called by tpci200_uninstall are in pair. However,
> > > > > tpci200_unregister has some cleanup operations not in the
> > > > > tpci200_register. So the error handling code of tpci200_pci_probe has
> > > > > many different double free issues.
> > > > >
> > > > > Fix this problem by moving those cleanup operations out of
> > > > > tpci200_unregister, into tpci200_pci_remove and reverting
> > > > > the previous commit 9272e5d0028d ("ipack/carriers/tpci200:
> > > > > Fix a double free in tpci200_pci_probe").
> > > > >
> > > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > > Fixes: 9272e5d0028d ("ipack/carriers/tpci200: Fix a double free in tpci200_pci_probe")
> > > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > > ---
> > > > > v1->v2: revise PATCH 2/3, 3/3, not depending on PATCH 1/3; move the
> > > > > location change of tpci_unregister into one separate patch;
> > > >
> > > > Also needs to go to the stable trees, right?
> > >
> > > Yes, this needs to go to the stable trees.
> >
> > Hi gregkh,
> >
> > Let me clarify more. In my series, PATCH 3/4 4/4 depends on PATCH 1/4
> > and PATCH 2/4. And also PATCH 2/4 depends on PATCH 1/4 as they are
> > closely related.
> >
> > But from your reply, the last 2 patches should not depend on the first
> > 2 patches. I don't quite understand as I don't send some patch series
> > before. For a patch series, the latter ones should depend on the
> > former ones, right? If I have any misunderstanding, please let me
> > know.
>
> Yes, they can depend on previous patches, but if some patches are to be
> merged today for 5.14-final, and others later for 5.15-rc1, then ideally
> they will be separate series of changes as those go to two different
> branches in my tree at the moment.
>
> > BTW, PATCH 3/4 has some compilation issues. I will fix it in the next version.
>
> As you haven't even tested these, I'm really hesitant to take them at
> all.
>
> Please just send the first two patches, fixed up, as a series after you
> have tested them, and then after they are merged into Linus's tree, you
> can send the cleanup patches, as they are just "nice" to have.
>

That's good. I will send the first two patches. After they are merged,
then I will send the rest patches.

> thanks,
>
> greg k-h