[Xen-devel] [PATCH] xen/blkback: fix memory leaks

Wenwen Wang posted 1 patch 6 days ago
Failed in applying to current master (apply log)
drivers/block/xen-blkback/xenbus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

[Xen-devel] [PATCH] xen/blkback: fix memory leaks

Posted by Wenwen Wang 6 days ago
In read_per_ring_refs(), after 'req' and related memory regions are
allocated, xen_blkif_map() is invoked to map the shared frame, irq, and
etc. However, if this mapping process fails, no cleanup is performed,
leading to memory leaks. To fix this issue, invoke the cleanup before
returning the error.

Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
 drivers/block/xen-blkback/xenbus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3ac6a5d..b90dbcd 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -965,6 +965,7 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
 		}
 	}
 
+	err = -ENOMEM;
 	for (i = 0; i < nr_grefs * XEN_BLKIF_REQS_PER_PAGE; i++) {
 		req = kzalloc(sizeof(*req), GFP_KERNEL);
 		if (!req)
@@ -987,7 +988,7 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
 	err = xen_blkif_map(ring, ring_ref, nr_grefs, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err, "mapping ring-ref port %u", evtchn);
-		return err;
+		goto fail;
 	}
 
 	return 0;
@@ -1007,8 +1008,7 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
 		}
 		kfree(req);
 	}
-	return -ENOMEM;
-
+	return err;
 }
 
 static int connect_ring(struct backend_info *be)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: fix memory leaks

Posted by Boris Ostrovsky 5 days ago
On 8/11/19 1:23 PM, Wenwen Wang wrote:
> In read_per_ring_refs(), after 'req' and related memory regions are
> allocated, xen_blkif_map() is invoked to map the shared frame, irq, and
> etc. However, if this mapping process fails, no cleanup is performed,
> leading to memory leaks. To fix this issue, invoke the cleanup before
> returning the error.


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: fix memory leaks

Posted by Jens Axboe 5 days ago
On 8/11/19 10:23 AM, Wenwen Wang wrote:
> In read_per_ring_refs(), after 'req' and related memory regions are
> allocated, xen_blkif_map() is invoked to map the shared frame, irq, and
> etc. However, if this mapping process fails, no cleanup is performed,
> leading to memory leaks. To fix this issue, invoke the cleanup before
> returning the error.

Applied, thanks.

-- 
Jens Axboe


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: fix memory leaks

Posted by Roger Pau Monné 5 days ago
On Sun, Aug 11, 2019 at 12:23:22PM -0500, Wenwen Wang wrote:
> In read_per_ring_refs(), after 'req' and related memory regions are
> allocated, xen_blkif_map() is invoked to map the shared frame, irq, and
> etc. However, if this mapping process fails, no cleanup is performed,
> leading to memory leaks. To fix this issue, invoke the cleanup before
> returning the error.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>

Thanks!

> ---
>  drivers/block/xen-blkback/xenbus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 3ac6a5d..b90dbcd 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -965,6 +965,7 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
>  		}
>  	}
>  
> +	err = -ENOMEM;
>  	for (i = 0; i < nr_grefs * XEN_BLKIF_REQS_PER_PAGE; i++) {
>  		req = kzalloc(sizeof(*req), GFP_KERNEL);
>  		if (!req)
> @@ -987,7 +988,7 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
>  	err = xen_blkif_map(ring, ring_ref, nr_grefs, evtchn);

You could also move the xen_blkif_map cal before the allocation loop,
since there's nothing in xen_blkif_map that uses the memory allocated
AFAICT, but I'm fine either way.

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel