[PATCH] x86: pci/xen, fix memory leak

[PATCH] x86: pci/xen, fix memory leak

[Jiri Slaby]

Stanse found that xen_setup_msi_irqs leaks memory when
xen_allocate_pirq fails. Free the memory in that fail path.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/pci/xen.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 117f5b8..d7b5109 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -147,8 +147,10 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 		irq = xen_allocate_pirq(v[i], 0, /* not sharable */
 			(type == PCI_CAP_ID_MSIX) ?
 			"pcifront-msi-x" : "pcifront-msi");
-		if (irq < 0)
-			return -1;
+		if (irq < 0) {
+			ret = -1;
+			goto free;
+		}
 
 		ret = set_irq_msi(irq, msidesc);
 		if (ret)
@@ -164,7 +166,7 @@ error:
 	if (ret == -ENODEV)
 		dev_err(&dev->dev, "Xen PCI frontend has not registered" \
 			" MSI/MSI-X support!\n");
-
+free:
 	kfree(v);
 	return ret;
 }
-- 
1.7.3.1

[PATCH] Char: virtio_console, fix memory leak

[Jiri Slaby]

Stanse found that in init_vqs, memory is leaked under certain
circumstanses (the fail path order is incorrect). Fix that by checking
allocations in one turn and free all of them at once if some fails
(some may be NULL, but this is OK).

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/char/virtio_console.c |   37 +++++++++----------------------------
 1 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6c1b676..896a2ce 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1547,31 +1547,16 @@ static int init_vqs(struct ports_device *portdev)
 	nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
 
 	vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
-	if (!vqs) {
-		err = -ENOMEM;
-		goto fail;
-	}
 	io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL);
-	if (!io_callbacks) {
-		err = -ENOMEM;
-		goto free_vqs;
-	}
 	io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL);
-	if (!io_names) {
-		err = -ENOMEM;
-		goto free_callbacks;
-	}
 	portdev->in_vqs = kmalloc(nr_ports * sizeof(struct virtqueue *),
 				  GFP_KERNEL);
-	if (!portdev->in_vqs) {
-		err = -ENOMEM;
-		goto free_names;
-	}
 	portdev->out_vqs = kmalloc(nr_ports * sizeof(struct virtqueue *),
 				   GFP_KERNEL);
-	if (!portdev->out_vqs) {
+	if (!vqs || !io_callbacks || !io_names || !portdev->in_vqs ||
+			!portdev->out_vqs) {
 		err = -ENOMEM;
-		goto free_invqs;
+		goto free;
 	}
 
 	/*
@@ -1605,7 +1590,7 @@ static int init_vqs(struct ports_device *portdev)
 					      io_callbacks,
 					      (const char **)io_names);
 	if (err)
-		goto free_outvqs;
+		goto free;
 
 	j = 0;
 	portdev->in_vqs[0] = vqs[0];
@@ -1621,23 +1606,19 @@ static int init_vqs(struct ports_device *portdev)
 			portdev->out_vqs[i] = vqs[j + 1];
 		}
 	}
-	kfree(io_callbacks);
 	kfree(io_names);
+	kfree(io_callbacks);
 	kfree(vqs);
 
 	return 0;
 
-free_names:
-	kfree(io_names);
-free_callbacks:
-	kfree(io_callbacks);
-free_outvqs:
+free:
 	kfree(portdev->out_vqs);
-free_invqs:
 	kfree(portdev->in_vqs);
-free_vqs:
+	kfree(io_names);
+	kfree(io_callbacks);
 	kfree(vqs);
-fail:
+
 	return err;
 }
 
-- 
1.7.3.1

Re: [PATCH] Char: virtio_console, fix memory leak

[Rusty Russell]

On Sat, 6 Nov 2010 07:36:50 pm Jiri Slaby wrote:
> Stanse found that in init_vqs, memory is leaked under certain
> circumstanses (the fail path order is incorrect). Fix that by checking
> allocations in one turn and free all of them at once if some fails
> (some may be NULL, but this is OK).

Good catch, thanks!

I generally prefer the static rather than lazy method of cleanup, but
in this case it's ugly both before and after...

Applied,
Rusty.

[PATCH] IDE: ide-floppy, remove unnecessary NULL check

[Jiri Slaby]

Stanse founf that rq in ide_floppy_callback cannot be NULL, because it
is dereferenced all around. So remove the superfluous check.

This appeared after blk_* macors removal.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org
---
 drivers/ide/ide-floppy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 5406b6e..536ff68 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -73,7 +73,7 @@ static int ide_floppy_callback(ide_drive_t *drive, int dsc)
 		drive->failed_pc = NULL;
 
 	if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
-	    (rq && rq->cmd_type == REQ_TYPE_BLOCK_PC))
+	    rq->cmd_type == REQ_TYPE_BLOCK_PC)
 		uptodate = 1; /* FIXME */
 	else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
 
-- 
1.7.3.1

Re: [PATCH] IDE: ide-floppy, remove unnecessary NULL check

[Sergei Shtylyov]

Hello.

On 06-11-2010 12:06, Jiri Slaby wrote:

> Stanse founf that rq in ide_floppy_callback cannot be NULL, because it

    found?

> is dereferenced all around. So remove the superfluous check.

> This appeared after blk_* macors removal.

    macros?

> Signed-off-by: Jiri Slaby<jslaby@suse.cz>
> Cc: Christoph Hellwig<hch@lst.de>
> Cc: "David S. Miller"<davem@davemloft.net>
> Cc: linux-ide@vger.kernel.org

MBR, Sergei

Re: [PATCH] IDE: ide-floppy, remove unnecessary NULL check

[Borislav Petkov]

On Sat, Nov 06, 2010 at 10:06:51AM +0100, Jiri Slaby wrote:
> Stanse founf that rq in ide_floppy_callback cannot be NULL, because it
> is dereferenced all around. So remove the superfluous check.
> 
> This appeared after blk_* macors removal.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-ide@vger.kernel.org
> ---
>  drivers/ide/ide-floppy.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 5406b6e..536ff68 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -73,7 +73,7 @@ static int ide_floppy_callback(ide_drive_t *drive, int dsc)
>  		drive->failed_pc = NULL;
>  
>  	if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
> -	    (rq && rq->cmd_type == REQ_TYPE_BLOCK_PC))
> +	    rq->cmd_type == REQ_TYPE_BLOCK_PC)
>  		uptodate = 1; /* FIXME */
>  	else if (pc->c[0] == GPCMD_REQUEST_SENSE) {

If this isn't a fix for a real bug, then there's no need for it since
IDE is deprecated and thus in bugfix mode only.

-- 
Regards/Gruss,
    Boris.

Re: [PATCH] IDE: ide-floppy, remove unnecessary NULL check

[David Miller]

From: Jiri Slaby <jslaby@suse.cz>
Date: Sat,  6 Nov 2010 10:06:51 +0100

> Stanse founf that rq in ide_floppy_callback cannot be NULL, because it
> is dereferenced all around. So remove the superfluous check.
> 
> This appeared after blk_* macors removal.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Applied with commit message typos fixed.

Thanks.

[PATCH] ocfs2: fix memory leak

[Jiri Slaby]

Stanse found that o2hb_heartbeat_group_make_item leaks some memory on
fail paths. Fix the paths by adding a new label and jump there.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: ocfs2-devel@oss.oracle.com
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/ocfs2/cluster/heartbeat.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 52c7557..9f26ac9 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1964,8 +1964,10 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
 	if (reg == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	if (strlen(name) > O2HB_MAX_REGION_NAME_LEN)
-		return ERR_PTR(-ENAMETOOLONG);
+	if (strlen(name) > O2HB_MAX_REGION_NAME_LEN) {
+		ret = -ENAMETOOLONG;
+		goto free;
+	}
 
 	spin_lock(&o2hb_live_lock);
 	reg->hr_region_num = 0;
@@ -1974,7 +1976,8 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
 							 O2NM_MAX_REGIONS);
 		if (reg->hr_region_num >= O2NM_MAX_REGIONS) {
 			spin_unlock(&o2hb_live_lock);
-			return ERR_PTR(-EFBIG);
+			ret = -EFBIG;
+			goto free;
 		}
 		set_bit(reg->hr_region_num, o2hb_region_bitmap);
 	}
@@ -1986,10 +1989,13 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
 	ret = o2hb_debug_region_init(reg, o2hb_debug_dir);
 	if (ret) {
 		config_item_put(&reg->hr_item);
-		return ERR_PTR(ret);
+		goto free;
 	}
 
 	return &reg->hr_item;
+free:
+	kfree(reg);
+	return ERR_PTR(ret);
 }
 
 static void o2hb_heartbeat_group_drop_item(struct config_group *group,
-- 
1.7.3.1

Re: [PATCH] ocfs2: fix memory leak

[Joel Becker]

On Sat, Nov 06, 2010 at 10:06:52AM +0100, Jiri Slaby wrote:
> Stanse found that o2hb_heartbeat_group_make_item leaks some memory on
> fail paths. Fix the paths by adding a new label and jump there.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Mark Fasheh <mfasheh@suse.com>
> Cc: Joel Becker <joel.becker@oracle.com>
> Cc: ocfs2-devel@oss.oracle.com
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: linux-fsdevel@vger.kernel.org

	This patch is now in the fixes branch of ocfs2.git.

Joel


-- 

"In a crisis, don't hide behind anything or anybody. They're going
 to find you anyway."
	- Paul "Bear" Bryant

Joel Becker
Senior Development Manager
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] x86: pci/xen, fix memory leak

[Konrad Rzeszutek Wilk]

On Sat, Nov 06, 2010 at 10:06:49AM +0100, Jiri Slaby wrote:
> Stanse found that xen_setup_msi_irqs leaks memory when
> xen_allocate_pirq fails. Free the memory in that fail path.

Duh! Yes, sticking it in the queue for bug-fixes. Thank you
for finding it.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> ---
>  arch/x86/pci/xen.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 117f5b8..d7b5109 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -147,8 +147,10 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  		irq = xen_allocate_pirq(v[i], 0, /* not sharable */
>  			(type == PCI_CAP_ID_MSIX) ?
>  			"pcifront-msi-x" : "pcifront-msi");
> -		if (irq < 0)
> -			return -1;
> +		if (irq < 0) {
> +			ret = -1;
> +			goto free;
> +		}
>  
>  		ret = set_irq_msi(irq, msidesc);
>  		if (ret)
> @@ -164,7 +166,7 @@ error:
>  	if (ret == -ENODEV)
>  		dev_err(&dev->dev, "Xen PCI frontend has not registered" \
>  			" MSI/MSI-X support!\n");
> -
> +free:
>  	kfree(v);
>  	return ret;
>  }
> -- 
> 1.7.3.1
>