[PATCH 01/38] sh: dma-sysfs: add missing put_device call

[PATCH 01/38] sh: dma-sysfs: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/sh/drivers/dma/dma-sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/sh/drivers/dma/dma-sysfs.c b/arch/sh/drivers/dma/dma-sysfs.c
index 4b15fed..76271e6 100644
--- a/arch/sh/drivers/dma/dma-sysfs.c
+++ b/arch/sh/drivers/dma/dma-sysfs.c
@@ -131,8 +131,10 @@ int dma_create_sysfs_files(struct dma_channel *chan, struct dma_info *info)
 	dev->bus = &dma_subsys;
 
 	ret = device_register(dev);
-	if (ret)
+	if (ret) {
+		put_device(dev);
 		return ret;
+	}
 
 	ret |= device_create_file(dev, &dev_attr_dev_id);
 	ret |= device_create_file(dev, &dev_attr_count);
-- 
1.8.3.1

[PATCH 02/38] powerpc: qe_lib: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/powerpc/sysdev/qe_lib/qe_ic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index b2b87c3..2e15262 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -493,6 +493,7 @@ static int __init init_qe_ic_sysfs(void)
 	rc = device_register(&device_qe_ic);
 	if (rc) {
 		printk(KERN_ERR "Failed registering qe_ic sys device\n");
+		put_device(&device_qe_ic);
 		return -ENODEV;
 	}
 	return 0;
-- 
1.8.3.1

[PATCH 03/38] powerpc: cell: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/powerpc/platforms/cell/spu_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
index f85db3a..f7fb0d9 100644
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -597,6 +597,7 @@ static int spu_create_dev(struct spu *spu)
 	if (ret) {
 		printk(KERN_ERR "Can't register SPU %d with sysfs\n",
 				spu->number);
+		put_device(&spu->dev);
 		return ret;
 	}
 
-- 
1.8.3.1

[PATCH 04/38] powerpc: kernel: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/powerpc/kernel/vio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index e7d0c88f..e8ed87e 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1494,6 +1494,7 @@ static int __init vio_bus_init(void)
 	if (err) {
 		printk(KERN_WARNING "%s: device_register returned %i\n",
 				__func__, err);
+		put_device(&vio_bus_device.dev);
 		return err;
 	}
 
-- 
1.8.3.1

[PATCH 05/38] sparc: kernel: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
The kfree() has been removed because put_device will set the underlying kref's
reference count to zero and hence vio_dev_release will get called resulting in
kfreeing the structure.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/sparc/kernel/vio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index 8647fcc..fb0088d 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -307,7 +307,7 @@ static struct vio_dev *vio_create_one(struct mdesc_handle *hp, u64 mp,
 	if (err) {
 		printk(KERN_ERR "VIO: Could not register device %s, err=%d\n",
 		       dev_name(&vdev->dev), err);
-		kfree(vdev);
+		put_device(&vdev->dev);
 		return NULL;
 	}
 	if (vdev->dp)
-- 
1.8.3.1

[PATCH 06/38] arm: mach-s3c64: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-s3c64xx/dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-s3c64xx/dma.c b/arch/arm/mach-s3c64xx/dma.c
index 7e22c21..090852d 100644
--- a/arch/arm/mach-s3c64xx/dma.c
+++ b/arch/arm/mach-s3c64xx/dma.c
@@ -663,6 +663,7 @@ static int s3c64xx_dma_init1(int chno, enum dma_ch chbase,
 	err = device_register(&dmac->dev);
 	if (err) {
 		printk(KERN_ERR "%s: failed to register device\n", __func__);
+		put_device(&dmac->dev);
 		goto err_alloc;
 	}
 
-- 
1.8.3.1

[PATCH 07/38] arm: locomo: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Also, rework error path to remove the out label by return -ENOMEM explicitly if
kzalloc fails.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/common/locomo.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index b55c362..e17d02c 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -224,10 +224,8 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 	int ret;
 
 	dev = kzalloc(sizeof(struct locomo_dev), GFP_KERNEL);
-	if (!dev) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!dev)
+		return -ENOMEM;
 
 	/*
 	 * If the parent device has a DMA mask associated with it,
@@ -256,8 +254,7 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info)
 
 	ret = device_register(&dev->dev);
 	if (ret) {
- out:
-		kfree(dev);
+ 		put_device(&dev->dev);
 	}
 	return ret;
 }
-- 
1.8.3.1

[PATCH 08/38] arm: mach-rpc: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-rpc/ecard.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rpc/ecard.c b/arch/arm/mach-rpc/ecard.c
index fcb1d59..1c5f7cb 100644
--- a/arch/arm/mach-rpc/ecard.c
+++ b/arch/arm/mach-rpc/ecard.c
@@ -961,8 +961,10 @@ static int __init ecard_probe(int slot, unsigned irq, card_type_t type)
 	slot_to_expcard[slot] = ec;
 
 	rc = device_register(&ec->dev);
-	if (rc)
+	if (rc) {
+		put_device(&ec->dev);
 		goto nodev;
+	}
 
 	return 0;
 
-- 
1.8.3.1

[PATCH 09/38] arm: mach-imx: add missing put_device calls

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-imx/devices/devices.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices.c b/arch/arm/mach-imx/devices/devices.c
index 1b4366a..0b17c89 100644
--- a/arch/arm/mach-imx/devices/devices.c
+++ b/arch/arm/mach-imx/devices/devices.c
@@ -37,11 +37,14 @@ int __init mxc_device_init(void)
 	int ret;
 
 	ret = device_register(&mxc_aips_bus);
-	if (ret < 0)
+	if (ret < 0) {
+		put_device(&mxc_aips_bus);
 		goto done;
+	}
 
 	ret = device_register(&mxc_ahb_bus);
-
+	if(ret < 0)
+		put_device(&mxc_ahb_bus);
 done:
 	return ret;
 }
-- 
1.8.3.1

[PATCH 10/38] arm: exynos: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-exynos/common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 61d2906..8b58829 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -407,7 +407,13 @@ early_initcall(exynos4_l2x0_cache_init);
 
 static int __init exynos_init(void)
 {
+	int rc = 0;
+
 	printk(KERN_INFO "EXYNOS: Initializing architecture\n");
 
-	return device_register(&exynos4_dev);
+	rc = device_register(&exynos4_dev);
+	if (rc)
+		put_device(&exynos4_dev);
+	
+	return rc;
 }
-- 
1.8.3.1

[PATCH 11/38] arm: mach-integrator: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Calling put_device on the failed device will allow lm_device_release to be
called which will kfree() the struct lm_device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-integrator/lm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-integrator/lm.c b/arch/arm/mach-integrator/lm.c
index f52c7af..1199f38 100644
--- a/arch/arm/mach-integrator/lm.c
+++ b/arch/arm/mach-integrator/lm.c
@@ -89,8 +89,10 @@ int lm_device_register(struct lm_device *dev)
 	ret = request_resource(&iomem_resource, &dev->resource);
 	if (ret == 0) {
 		ret = device_register(&dev->dev);
-		if (ret)
+		if (ret) {
 			release_resource(&dev->resource);
+			put_device(&dev->dev);
+		}
 	}
 	return ret;
 }
-- 
1.8.3.1

[PATCH 12/38] arm: s5pv210: add missing put_device call

[Levente Kurusa]

This is required so that give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/arm/mach-s5pv210/common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-s5pv210/common.c b/arch/arm/mach-s5pv210/common.c
index 26027a2..81f43ae 100644
--- a/arch/arm/mach-s5pv210/common.c
+++ b/arch/arm/mach-s5pv210/common.c
@@ -267,8 +267,14 @@ core_initcall(s5pv210_core_init);
 
 int __init s5pv210_init(void)
 {
+	int rc = 0;
+
 	printk(KERN_INFO "S5PV210: Initializing architecture\n");
-	return device_register(&s5pv210_dev);
+	rc = device_register(&s5pv210_dev);
+	if (rc)
+		put_device(&s5pv210_dev);
+
+	return rc;
 }
 
 /* uart registration process */
-- 
1.8.3.1

[PATCH 13/38] arm: s390: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/s390/kernel/time.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 064c308..16c717f 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -1351,8 +1351,10 @@ static int __init etr_register_port(struct device *dev)
 	int rc;
 
 	rc = device_register(dev);
-	if (rc)
+	if (rc) {
+		put_device(dev);
 		goto out;
+	}
 	for (attr = etr_port_attributes; *attr; attr++) {
 		rc = device_create_file(dev, *attr);
 		if (rc)
-- 
1.8.3.1

[PATCH 14/38] mips: txx9: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Also, rework error path so that it is easier to read.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/mips/txx9/generic/setup.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index 2b0b83c..24332f5 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -937,6 +937,12 @@ static ssize_t txx9_sram_write(struct file *filp, struct kobject *kobj,
 	return size;
 }
 
+void txx9_device_release(struct device *dev) {
+	struct txx9_sramc_dev *sramc_dev;
+	txx9_sramc_dev = container_of(dev, struct txx9_sramc_dev, dev);
+	kfree(txx9_sramc_dev);
+}
+
 void __init txx9_sramc_init(struct resource *r)
 {
 	struct txx9_sramc_dev *dev;
@@ -951,8 +957,11 @@ void __init txx9_sramc_init(struct resource *r)
 		return;
 	size = resource_size(r);
 	dev->base = ioremap(r->start, size);
-	if (!dev->base)
-		goto exit;
+	if (!dev->base) {
+		kfree(dev);
+		return;
+	}
+	dev->dev.release = &txx9_device_release;
 	dev->dev.bus = &txx9_sramc_subsys;
 	sysfs_bin_attr_init(&dev->bindata_attr);
 	dev->bindata_attr.attr.name = "bindata";
@@ -963,17 +972,15 @@ void __init txx9_sramc_init(struct resource *r)
 	dev->bindata_attr.private = dev;
 	err = device_register(&dev->dev);
 	if (err)
-		goto exit;
+		goto exit_put;
 	err = sysfs_create_bin_file(&dev->dev.kobj, &dev->bindata_attr);
 	if (err) {
 		device_unregister(&dev->dev);
-		goto exit;
-	}
-	return;
-exit:
-	if (dev) {
-		if (dev->base)
-			iounmap(dev->base);
+		iounmap(dev->base);
 		kfree(dev);
 	}
+	return;
+exit_put:
+	put_device(&dev->dev);
+	return;
 }
-- 
1.8.3.1

[PATCH 15/38] mips: txx9: 7segled: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Also, add a new tx_7segled_release function which will be called after the
put_device to ensure that device is kfree'd.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/mips/txx9/generic/7segled.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/mips/txx9/generic/7segled.c b/arch/mips/txx9/generic/7segled.c
index 4642f56..3caa548 100644
--- a/arch/mips/txx9/generic/7segled.c
+++ b/arch/mips/txx9/generic/7segled.c
@@ -83,6 +83,10 @@ static struct bus_type tx_7segled_subsys = {
 	.dev_name	= "7segled",
 };
 
+void tx_7segled_release(struct device *dev) {
+	kfree(dev);
+}
+
 static int __init tx_7segled_init_sysfs(void)
 {
 	int error, i;
@@ -103,11 +107,14 @@ static int __init tx_7segled_init_sysfs(void)
 		}
 		dev->id = i;
 		dev->bus = &tx_7segled_subsys;
+		dev->release = &tx_7segled_release;
 		error = device_register(dev);
-		if (!error) {
-			device_create_file(dev, &dev_attr_ascii);
-			device_create_file(dev, &dev_attr_raw);
+		if (error) {
+			put_device(dev);
+			return error;
 		}
+		device_create_file(dev, &dev_attr_ascii);
+		device_create_file(dev, &dev_attr_raw);	
 	}
 	return error;
 }
-- 
1.8.3.1

[PATCH 16/38] mips: sgi-ip22: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Also, create a gio_bus_release() that calls kfree on the device argument to
properly kfree() the memory allocated for the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/mips/sgi-ip22/ip22-gio.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/mips/sgi-ip22/ip22-gio.c b/arch/mips/sgi-ip22/ip22-gio.c
index ab0e379..931da33 100644
--- a/arch/mips/sgi-ip22/ip22-gio.c
+++ b/arch/mips/sgi-ip22/ip22-gio.c
@@ -24,8 +24,13 @@ static struct {
 	{ .name = "SGI GR2/GR3", .id = 0x7f },
 };
 
+void gio_bus_release(struct device *dev) {
+	kfree(dev);
+}
+
 static struct device gio_bus = {
 	.init_name = "gio",
+	.release = &gio_bus_release,
 };
 
 /**
@@ -400,8 +405,10 @@ int __init ip22_gio_init(void)
 	int ret;
 
 	ret = device_register(&gio_bus);
-	if (ret)
+	if (ret) {
+		put_device(&gio_bus);
 		return ret;
+	}
 
 	ret = bus_register(&gio_bus_type);
 	if (!ret) {
-- 
1.8.3.1

[PATCH 17/38] parisc: kernel: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/parisc/kernel/drivers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/drivers.c b/arch/parisc/kernel/drivers.c
index 14285ca..b97cc05 100644
--- a/arch/parisc/kernel/drivers.c
+++ b/arch/parisc/kernel/drivers.c
@@ -428,7 +428,7 @@ struct parisc_device * create_tree_node(char id, struct device *parent)
 	dev->dev.dma_mask = &dev->dma_mask;
 	dev->dev.coherent_dma_mask = dev->dma_mask;
 	if (device_register(&dev->dev)) {
-		kfree(dev);
+		put_device(&dev->dev);
 		return NULL;
 	}
 
-- 
1.8.3.1

[PATCH 18/38] ia64: kernel: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Removed kfree() as put_device() will result in tiocx_bus_release called.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 arch/ia64/sn/kernel/tiocx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/sn/kernel/tiocx.c b/arch/ia64/sn/kernel/tiocx.c
index e35f648..6f369cc 100644
--- a/arch/ia64/sn/kernel/tiocx.c
+++ b/arch/ia64/sn/kernel/tiocx.c
@@ -209,7 +209,7 @@ cx_device_register(nasid_t nasid, int part_num, int mfg_num,
 	dev_set_name(&cx_dev->dev, "%d", cx_dev->cx_id.nasid);
 	r = device_register(&cx_dev->dev);
 	if (r) {
-		kfree(cx_dev);
+		put_device(&cx_dev->dev);
 		return r;
 	}
 	get_device(&cx_dev->dev);
-- 
1.8.3.1

[PATCH 19/38] net: atm: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 net/atm/atm_sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
index 350bf62..0e070c2 100644
--- a/net/atm/atm_sysfs.c
+++ b/net/atm/atm_sysfs.c
@@ -154,8 +154,10 @@ int atm_register_sysfs(struct atm_dev *adev, struct device *parent)
 
 	dev_set_name(cdev, "%s%d", adev->type, adev->number);
 	err = device_register(cdev);
-	if (err < 0)
+	if (err < 0) {
+		put_device(cdev);
 		return err;
+	}
 
 	for (i = 0; atm_attrs[i]; i++) {
 		err = device_create_file(cdev, atm_attrs[i]);
-- 
1.8.3.1

[PATCH 20/38] net: iucv: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 net/iucv/af_iucv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 168aff5..0096206 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -2377,8 +2377,10 @@ static int afiucv_iucv_init(void)
 	af_iucv_dev->release = (void (*)(struct device *))kfree;
 	af_iucv_dev->driver = &af_iucv_driver;
 	err = device_register(af_iucv_dev);
-	if (err)
+	if (err) {
+		put_device(af_iucv_dev);
 		goto out_driver;
+	}
 	return 0;
 
 out_driver:
-- 
1.8.3.1

[PATCH 21/38] workqueue: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Remove the kfree() as that is the job of wq_device_release which will now
be called due to the reference count actually reaching zero.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 kernel/workqueue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 987293d..f3b3398 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3361,7 +3361,7 @@ int workqueue_sysfs_register(struct workqueue_struct *wq)
 
 	ret = device_register(&wq_dev->dev);
 	if (ret) {
-		kfree(wq_dev);
+		put_device(&wq_dev->dev);
 		wq->wq_dev = NULL;
 		return ret;
 	}
-- 
1.8.3.1

[PATCH 22/38] sound: ac97: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 sound/pci/ac97/ac97_codec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
index bf578ba2..55186f8 100644
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -1964,6 +1964,7 @@ static int snd_ac97_dev_register(struct snd_device *device)
 		     snd_ac97_get_short_name(ac97));
 	if ((err = device_register(&ac97->dev)) < 0) {
 		snd_printk(KERN_ERR "Can't register ac97 bus\n");
+		put_device(&ac97->dev);
 		ac97->dev.bus = NULL;
 		return err;
 	}
-- 
1.8.3.1

[PATCH 23/38] sound: soc: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 sound/soc/soc-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 4e53d87..21caa09 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -536,6 +536,7 @@ static int soc_ac97_dev_register(struct snd_soc_codec *codec)
 	err = device_register(&codec->ac97->dev);
 	if (err < 0) {
 		dev_err(codec->dev, "ASoC: Can't register ac97 bus\n");
+		put_device(&codec->ac97->dev);
 		codec->ac97->dev.bus = NULL;
 		return err;
 	}
-- 
1.8.3.1

[PATCH 24/38] pcmcia: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/pcmcia/cs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c
index 5292db6..5057de9 100644
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -607,6 +607,7 @@ static int pccardd(void *__skt)
 	if (ret) {
 		dev_printk(KERN_WARNING, &skt->dev,
 			   "PCMCIA: unable to register socket\n");
+		put_device(&skt->dev);
 		skt->thread = NULL;
 		complete(&skt->thread_done);
 		return 0;
-- 
1.8.3.1

[PATCH 25/38] pnp: card: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/pnp/card.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pnp/card.c b/drivers/pnp/card.c
index bc00693..874c236 100644
--- a/drivers/pnp/card.c
+++ b/drivers/pnp/card.c
@@ -239,6 +239,7 @@ int pnp_add_card(struct pnp_card *card)
 	error = device_register(&card->dev);
 	if (error) {
 		dev_err(&card->dev, "could not register (err=%d)\n", error);
+		put_device(&card->dev);
 		return error;
 	}
 
-- 
1.8.3.1

[PATCH 26/38] xen: xenbus: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/xen/xenbus/xenbus_probe.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 3c0a74b..4abb9ee 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -465,8 +465,10 @@ int xenbus_probe_node(struct xen_bus_type *bus,
 
 	/* Register with generic device framework. */
 	err = device_register(&xendev->dev);
-	if (err)
+	if (err) {
+		put_device(&xendev->dev);
 		goto fail;
+	}
 
 	return 0;
 fail:
-- 
1.8.3.1

[PATCH 27/38] superhyway: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Also, this makes superhyway_device_release actually used.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/sh/superhyway/superhyway.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/sh/superhyway/superhyway.c b/drivers/sh/superhyway/superhyway.c
index 2d9e7f3..765cf7d 100644
--- a/drivers/sh/superhyway/superhyway.c
+++ b/drivers/sh/superhyway/superhyway.c
@@ -54,6 +54,7 @@ int superhyway_add_device(unsigned long base, struct superhyway_device *sdev,
 			  struct superhyway_bus *bus)
 {
 	struct superhyway_device *dev = sdev;
+	int rc = 0;
 
 	if (!dev) {
 		dev = kzalloc(sizeof(struct superhyway_device), GFP_KERNEL);
@@ -86,8 +87,11 @@ int superhyway_add_device(unsigned long base, struct superhyway_device *sdev,
 	dev_set_name(&dev->dev, "%02x", superhyway_devices);
 
 	superhyway_devices++;
+	rc = device_register(&dev->dev);
+	if (rc)
+		put_device(&dev->dev);
 
-	return device_register(&dev->dev);
+	return rc;
 }
 
 int superhyway_add_devices(struct superhyway_bus *bus,
-- 
1.8.3.1

[PATCH 28/38] superhyway: intc: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/sh/intc/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
index 8f32a13..c89768e 100644
--- a/drivers/sh/intc/core.c
+++ b/drivers/sh/intc/core.c
@@ -504,8 +504,10 @@ static int __init register_intc_devs(void)
 			if (error == 0)
 				error = device_create_file(&d->dev,
 							   &dev_attr_name);
-			if (error)
+			if (error) {
+				put_device(&d->dev);
 				break;
+			}
 		}
 	}
 
-- 
1.8.3.1

[PATCH 29/38] superhyway: maple: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Also rework maple_release_device to use maple_free_dev instead, avoiding
same code being in two functions at the same time.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/sh/maple/maple.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index bec81c2..c4afb7e 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -134,16 +134,18 @@ static int maple_dma_done(void)
 	return (__raw_readl(MAPLE_STATE) & 1) == 0;
 }
 
+static void maple_free_dev(struct maple_device *mdev)
+{
+	kmem_cache_free(maple_queue_cache, mdev->mq->recvbuf);
+	kfree(mdev->mq);
+	kfree(mdev);
+}
+
 static void maple_release_device(struct device *dev)
 {
 	struct maple_device *mdev;
-	struct mapleq *mq;
-
 	mdev = to_maple_dev(dev);
-	mq = mdev->mq;
-	kmem_cache_free(maple_queue_cache, mq->recvbuf);
-	kfree(mq);
-	kfree(mdev);
+	maple_free_dev(mdev);
 }
 
 /**
@@ -234,13 +236,6 @@ static struct maple_device *maple_alloc_dev(int port, int unit)
 	return mdev;
 }
 
-static void maple_free_dev(struct maple_device *mdev)
-{
-	kmem_cache_free(maple_queue_cache, mdev->mq->recvbuf);
-	kfree(mdev->mq);
-	kfree(mdev);
-}
-
 /* process the command queue into a maple command block
  * terminating command has bit 32 of first long set to 0
  */
@@ -393,7 +388,7 @@ static void maple_attach_driver(struct maple_device *mdev)
 		dev_warn(&mdev->dev, "could not register device at"
 			" (%d, %d), with error 0x%X\n", mdev->unit,
 			mdev->port, error);
-		maple_free_dev(mdev);
+		put_device(&mdev->dev);
 		mdev = NULL;
 		return;
 	}
-- 
1.8.3.1

[PATCH 30/38] vlynq: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
This will result in vlynq_device_release being called making the kfree()
in the errorpath unneccessary.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/vlynq/vlynq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/vlynq/vlynq.c b/drivers/vlynq/vlynq.c
index 7b07135..71d5905 100644
--- a/drivers/vlynq/vlynq.c
+++ b/drivers/vlynq/vlynq.c
@@ -748,11 +748,14 @@ static int vlynq_probe(struct platform_device *pdev)
 	return 0;
 
 fail_register:
+	put_device(&dev->dev);
+	dev->dev = 0;
 	iounmap(dev->local);
 fail_remap:
 fail_request:
 	release_mem_region(regs_res->start, len);
-	kfree(dev);
+	if(dev)
+		kfree(dev);
 	return result;
 }
 
-- 
1.8.3.1

Re: [PATCH 21/38] workqueue: add missing put_device call

[Tejun Heo]

On Thu, Dec 19, 2013 at 04:03:32PM +0100, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.
> Remove the kfree() as that is the job of wq_device_release which will now
> be called due to the reference count actually reaching zero.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
>  kernel/workqueue.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 987293d..f3b3398 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -3361,7 +3361,7 @@ int workqueue_sysfs_register(struct workqueue_struct *wq)
>  
>  	ret = device_register(&wq_dev->dev);
>  	if (ret) {
> -		kfree(wq_dev);
> +		put_device(&wq_dev->dev);

Umm... this doesn't look right.  You're basically converting the code
to the following,

	x = kmalloc();
	if (register(x) < 0)
		put(x)

They're not symmetrical anymore.  register(), or any API call really,
isn't supposed to have side effects which need explicit cleanup after
a failure.  The fact that x is properly initialized even after
register(x) failed is a coincidental implementation detail which
shouldn't be depended upon.  Your patch is actively breaking the
convention for no good reason.

 Nacked-by: Tejun Heo <tj@kernel.org>

>From the patch title, I suppose you posted a bunch of patches towards
this direction.  Please consider all of them nacked if they're doing
the same thing.

Thanks.

-- 
tejun

Re: [PATCH 17/38] parisc: kernel: add missing put_device call

[James Bottomley]

On Thu, 2013-12-19 at 16:03 +0100, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.

No, it's not.  And the subject is misleading at best, since this isn't
any form of problem in the code.

If the add fails, the device isn't visible in sysfs and no references
have been handed out.  We're perfectly entitled to follow the common
pattern of simply destroying it at this point rather than going through
the nascent refcounting.

James

Re: [PATCH 21/38] workqueue: add missing put_device call

[Levente Kurusa]

On 12/19/2013 04:20 PM, Tejun Heo wrote:
> Umm... this doesn't look right.  You're basically converting the code
> to the following,
> 
> 	x = kmalloc();
> 	if (register(x) < 0)
> 		put(x)
> 
> They're not symmetrical anymore.  register(), or any API call really,
> isn't supposed to have side effects which need explicit cleanup after
> a failure.  The fact that x is properly initialized even after
> register(x) failed is a coincidental implementation detail which
> shouldn't be depended upon.  Your patch is actively breaking the
> convention for no good reason.
> 
>  Nacked-by: Tejun Heo <tj@kernel.org>
> 
>>From the patch title, I suppose you posted a bunch of patches towards
> this direction.  Please consider all of them nacked if they're doing
> the same thing.
> 
> Thanks.
> 

The reason I removed the kfree() was because the put_device() will decrement
wq_dev->dev's reference count to zero (it is set to one by device_register) and hence the
wq_device_release() will be called. Now, this effectively does the same the kfree() call
would have done but also driver core is notified.

Also, if you take a look at the comment for the device_register() function, it explicitly
says NOT to kfree the struct device, but instead call put_device() and let the device's release()
function take care.

-- 
Regards,
Levente Kurusa

Re: [PATCH 21/38] workqueue: add missing put_device call

[Tejun Heo]

Hello,

On Thu, Dec 19, 2013 at 10:31 AM, Levente Kurusa <levex@linux.com> wrote:
> The reason I removed the kfree() was because the put_device() will decrement
> wq_dev->dev's reference count to zero (it is set to one by device_register) and hence the
> wq_device_release() will be called. Now, this effectively does the same the kfree() call
> would have done but also driver core is notified.

Yeah, I know it does the same thing. It's just not the right way to do it.

> Also, if you take a look at the comment for the device_register() function, it explicitly
> says NOT to kfree the struct device, but instead call put_device() and let the device's release()
> function take care.

Greg, the API as described by the comment is really weird and
unconventional. Failed calls are not supposed to have side effects
which require explicit cleanup. Can we please update the comment?

Thanks.

-- 
tejun

Re: [PATCH 21/38] workqueue: add missing put_device call

[Levente Kurusa]

[+Cc Greg]

On 12/19/2013 04:34 PM, Tejun Heo wrote:
> Hello,
> 
> On Thu, Dec 19, 2013 at 10:31 AM, Levente Kurusa <levex@linux.com> wrote:
>> The reason I removed the kfree() was because the put_device() will decrement
>> wq_dev->dev's reference count to zero (it is set to one by device_register) and hence the
>> wq_device_release() will be called. Now, this effectively does the same the kfree() call
>> would have done but also driver core is notified.
> 
> Yeah, I know it does the same thing. It's just not the right way to do it.
> 
>> Also, if you take a look at the comment for the device_register() function, it explicitly
>> says NOT to kfree the struct device, but instead call put_device() and let the device's release()
>> function take care.
> 
> Greg, the API as described by the comment is really weird and
> unconventional. Failed calls are not supposed to have side effects
> which require explicit cleanup. Can we please update the comment?
> 

Yes, it was already discussed that it would be more sane to have
device_register() call put_device() if it would fail, but Greg
said that the API was designed so that no kfree()s happen in the core.

-- 
Regards,
Levente Kurusa

Re: [PATCH 21/38] workqueue: add missing put_device call

[Tejun Heo]

On Thu, Dec 19, 2013 at 10:42 AM, Levente Kurusa <levex@linux.com> wrote:
>> Greg, the API as described by the comment is really weird and
>> unconventional. Failed calls are not supposed to have side effects
>> which require explicit cleanup. Can we please update the comment?
>>
>
> Yes, it was already discussed that it would be more sane to have
> device_register() call put_device() if it would fail, but Greg
> said that the API was designed so that no kfree()s happen in the core.

Of course not. For the third time: A failed call shouldn't make
material changes which makes difference to its caller. An obj which
failed init should be destructible in the exact reverse operation that
the caller took to create it. This is a fundamental rule of any API
design. There sure are occasional exceptions but there's no reason to
deviate from that here.

Thanks.

-- 
tejun

Re: [PATCH 22/38] sound: ac97: add missing put_device call

[Takashi Iwai]

At Thu, 19 Dec 2013 16:03:33 +0100,
Levente Kurusa wrote:
> 
> This is required so that we give up the last reference to the device.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>

Hmm, I see no merit by this patch.  It's an error path and the device
is referenced from nowhere, thus we don't care its refcount any longer
from there.  Any missing point?


thanks,

Takashi

> ---
>  sound/pci/ac97/ac97_codec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
> index bf578ba2..55186f8 100644
> --- a/sound/pci/ac97/ac97_codec.c
> +++ b/sound/pci/ac97/ac97_codec.c
> @@ -1964,6 +1964,7 @@ static int snd_ac97_dev_register(struct snd_device *device)
>  		     snd_ac97_get_short_name(ac97));
>  	if ((err = device_register(&ac97->dev)) < 0) {
>  		snd_printk(KERN_ERR "Can't register ac97 bus\n");
> +		put_device(&ac97->dev);
>  		ac97->dev.bus = NULL;
>  		return err;
>  	}
> -- 
> 1.8.3.1
> 

Re: [PATCH 30/38] vlynq: add missing put_device call

[Florian Fainelli]

2013/12/19 Levente Kurusa <levex@linux.com>:
> This is required so that we give up the last reference to the device.
> This will result in vlynq_device_release being called making the kfree()
> in the errorpath unneccessary.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>

Acked-by: Florian Fainelli <florian@openwrt.org>

> ---
>  drivers/vlynq/vlynq.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vlynq/vlynq.c b/drivers/vlynq/vlynq.c
> index 7b07135..71d5905 100644
> --- a/drivers/vlynq/vlynq.c
> +++ b/drivers/vlynq/vlynq.c
> @@ -748,11 +748,14 @@ static int vlynq_probe(struct platform_device *pdev)
>         return 0;
>
>  fail_register:
> +       put_device(&dev->dev);
> +       dev->dev = 0;
>         iounmap(dev->local);
>  fail_remap:
>  fail_request:
>         release_mem_region(regs_res->start, len);
> -       kfree(dev);
> +       if(dev)
> +               kfree(dev);

This probably now generates a checkpatch warning since it is safe to
call kfree() on a NULL pointer.

>         return result;
>  }
>
> --
> 1.8.3.1



-- 
Florian

Re: [PATCH 23/38] sound: soc: add missing put_device call

[Mark Brown]

On Thu, Dec 19, 2013 at 04:03:34PM +0100, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.

Applied, thanks.  Please use subject lines matching the standard for the
subsystem.

Re: [PATCH 19/38] net: atm: add missing put_device call

[David Miller]

From: Levente Kurusa <levex@linux.com>
Date: Thu, 19 Dec 2013 16:03:30 +0100

> This is required so that we give up the last reference to the device.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>
 ...
> @@ -154,8 +154,10 @@ int atm_register_sysfs(struct atm_dev *adev, struct device *parent)
>  
>  	dev_set_name(cdev, "%s%d", adev->type, adev->number);
>  	err = device_register(cdev);
> -	if (err < 0)
> +	if (err < 0) {
> +		put_device(cdev);
>  		return err;
> +	}

This doesn't make any sense.

Nothing has a reference to 'cdev'.  Yes, 'adev' has a reference,
acquired by the caller of this function, but that's not what we're
working with here.

Re: [PATCH 20/38] net: iucv: add missing put_device call

[David Miller]

From: Levente Kurusa <levex@linux.com>
Date: Thu, 19 Dec 2013 16:03:31 +0100

> This is required so that we give up the last reference to the device.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>
 ...
> @@ -2377,8 +2377,10 @@ static int afiucv_iucv_init(void)
>  	af_iucv_dev->release = (void (*)(struct device *))kfree;
>  	af_iucv_dev->driver = &af_iucv_driver;
>  	err = device_register(af_iucv_dev);
> -	if (err)
> +	if (err) {
> +		put_device(af_iucv_dev);
>  		goto out_driver;
> +	}

This has the same problem as the ATM patch, there is no reference held on
af_iucv_dev, I mean look at where it comes from:

	af_iucv_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
 ...
	dev_set_name(af_iucv_dev, "af_iucv");
	af_iucv_dev->bus = pr_iucv->bus;
	af_iucv_dev->parent = pr_iucv->root;
	af_iucv_dev->release = (void (*)(struct device *))kfree;
	af_iucv_dev->driver = &af_iucv_driver;

No reference count is incremented or set to one for af_iucv_dev.

Please study these code paths more carefully, thank you.

Re: [PATCH 05/38] sparc: kernel: add missing put_device call

[David Miller]

From: Levente Kurusa <levex@linux.com>
Date: Thu, 19 Dec 2013 16:03:16 +0100

> This is required so that we give up the last reference to the device.
> The kfree() has been removed because put_device will set the underlying kref's
> reference count to zero and hence vio_dev_release will get called resulting in
> kfreeing the structure.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>

There is no reference held to vdev->dev at this point, that's why
we should directly kfree() it and put_device() is not appropriate.

Pretty much all of your patches have this problem.

Re: [Xen-devel] [PATCH 26/38] xen: xenbus: add missing put_device call

[David Vrabel]

On 19/12/13 15:03, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
>  drivers/xen/xenbus/xenbus_probe.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> index 3c0a74b..4abb9ee 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -465,8 +465,10 @@ int xenbus_probe_node(struct xen_bus_type *bus,
>  
>  	/* Register with generic device framework. */
>  	err = device_register(&xendev->dev);
> -	if (err)
> +	if (err) {
> +		put_device(&xendev->dev);
>  		goto fail;
> +	}
>  
>  	return 0;
>  fail:

There is a kfree(xendev) here so this introduces a double-free.

David

Re: [PATCH 25/38] pnp: card: add missing put_device call

[Rafael J. Wysocki]

On Thursday, December 19, 2013 04:03:36 PM Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>

Queued up for 3.14, thanks!

> ---
>  drivers/pnp/card.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pnp/card.c b/drivers/pnp/card.c
> index bc00693..874c236 100644
> --- a/drivers/pnp/card.c
> +++ b/drivers/pnp/card.c
> @@ -239,6 +239,7 @@ int pnp_add_card(struct pnp_card *card)
>  	error = device_register(&card->dev);
>  	if (error) {
>  		dev_err(&card->dev, "could not register (err=%d)\n", error);
> +		put_device(&card->dev);
>  		return error;
>  	}
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [Xen-devel] [PATCH 26/38] xen: xenbus: add missing put_device call

[Ian Campbell]

On Thu, 2014-01-02 at 13:53 +0000, David Vrabel wrote:
> On 19/12/13 15:03, Levente Kurusa wrote:
> > This is required so that we give up the last reference to the device.
> > 
> > Signed-off-by: Levente Kurusa <levex@linux.com>
> > ---
> >  drivers/xen/xenbus/xenbus_probe.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> > index 3c0a74b..4abb9ee 100644
> > --- a/drivers/xen/xenbus/xenbus_probe.c
> > +++ b/drivers/xen/xenbus/xenbus_probe.c
> > @@ -465,8 +465,10 @@ int xenbus_probe_node(struct xen_bus_type *bus,
> >  
> >  	/* Register with generic device framework. */
> >  	err = device_register(&xendev->dev);
> > -	if (err)
> > +	if (err) {
> > +		put_device(&xendev->dev);
> >  		goto fail;
> > +	}
> >  
> >  	return 0;
> >  fail:
> 
> There is a kfree(xendev) here so this introduces a double-free.

How? put_device doesn't touch xendev itself, does it? It just drops the
ref on the dev member which is not separately dynamically allocated and
so not freed either.

Ian.

Re: [Xen-devel] [PATCH 26/38] xen: xenbus: add missing put_device call

[David Vrabel]

On 06/01/14 10:00, Ian Campbell wrote:
> On Thu, 2014-01-02 at 13:53 +0000, David Vrabel wrote:
>> On 19/12/13 15:03, Levente Kurusa wrote:
>>> This is required so that we give up the last reference to the device.
>>>
>>> Signed-off-by: Levente Kurusa <levex@linux.com>
>>> ---
>>>  drivers/xen/xenbus/xenbus_probe.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
>>> index 3c0a74b..4abb9ee 100644
>>> --- a/drivers/xen/xenbus/xenbus_probe.c
>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
>>> @@ -465,8 +465,10 @@ int xenbus_probe_node(struct xen_bus_type *bus,
>>>  
>>>  	/* Register with generic device framework. */
>>>  	err = device_register(&xendev->dev);
>>> -	if (err)
>>> +	if (err) {
>>> +		put_device(&xendev->dev);
>>>  		goto fail;
>>> +	}
>>>  
>>>  	return 0;
>>>  fail:
>>
>> There is a kfree(xendev) here so this introduces a double-free.
> 
> How? put_device doesn't touch xendev itself, does it? It just drops the
> ref on the dev member which is not separately dynamically allocated and
> so not freed either.

Releasing all references to the struct device frees the containing
structure via xenbus->dev.release. i.e., xenbus_dev_release() which does
kfree(xendev).

David

Re: [Xen-devel] [PATCH 26/38] xen: xenbus: add missing put_device call

[Ian Campbell]

On Mon, 2014-01-06 at 11:00 +0000, David Vrabel wrote:
> On 06/01/14 10:00, Ian Campbell wrote:
> > On Thu, 2014-01-02 at 13:53 +0000, David Vrabel wrote:
> >> On 19/12/13 15:03, Levente Kurusa wrote:
> >>> This is required so that we give up the last reference to the device.
> >>>
> >>> Signed-off-by: Levente Kurusa <levex@linux.com>
> >>> ---
> >>>  drivers/xen/xenbus/xenbus_probe.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> >>> index 3c0a74b..4abb9ee 100644
> >>> --- a/drivers/xen/xenbus/xenbus_probe.c
> >>> +++ b/drivers/xen/xenbus/xenbus_probe.c
> >>> @@ -465,8 +465,10 @@ int xenbus_probe_node(struct xen_bus_type *bus,
> >>>  
> >>>  	/* Register with generic device framework. */
> >>>  	err = device_register(&xendev->dev);
> >>> -	if (err)
> >>> +	if (err) {
> >>> +		put_device(&xendev->dev);
> >>>  		goto fail;
> >>> +	}
> >>>  
> >>>  	return 0;
> >>>  fail:
> >>
> >> There is a kfree(xendev) here so this introduces a double-free.
> > 
> > How? put_device doesn't touch xendev itself, does it? It just drops the
> > ref on the dev member which is not separately dynamically allocated and
> > so not freed either.
> 
> Releasing all references to the struct device frees the containing
> structure via xenbus->dev.release. i.e., xenbus_dev_release() which does
> kfree(xendev).

Ooh, twisty, well spotted ;-)

Ian.