Re: [Xen-devel] A memory leak problem in xen-blkback module

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

El 12/09/14 a les 8.58, Chentao(Boby) ha escrit:
> Hi Konrad,
> 
>     I find a memory leak problem in xen-blkback module of linux-3.14.4 
> release, and the newest 3.17-rc4 also has the same problem. The problem will 
> occur in below condition.
> 
>     In xen_blkbk_map function, first get_free_page from balloon or the list 
> of blkif free pages, then map this page. If get_free_page succeed, but map 
> failed,
> the grant handle corresponding to this page will be assigned to 
> BLKBACK_INVALID_HANDLE. Because map failed, it will execute xen_blkbk_unmap 
> to retrieve resources.
> But in xen_blkbk_unmap function, if the grant handle of a page is 
> BLKBACK_INVALID_HANDLE, it will continue to next loop to execute unmap and 
> put_free_pages.
> Only executes put_free_pages, these pages will be returned to the list of 
> blkif free pages and at last be returned to balloon.
> 
>     Make a summary, in the condition of get_free_page succeed but map failed, 
> the page will be leaked from balloon or the list of blkif free pages. I have 
> a immature thought,
> in xen_blkbk_unmap funtion, when judge the grant handle of a page is 
> BLKBACK_INVALID_HANDLE, can we execute put_free_pages to retrieve this one 
> page?
> 
>     Just like below:
>     if (pages[i]->handle == BLKBACK_INVALID_HANDLE) {
>         put_free_pages(blkif, pages[i]->page, 1);

This is not correct, and will fail to compile AFAICT. put_free_pages 
expects an array of pointers to page structs and you are passing a 
pointer to a page struct.

I have the following patch, which I think solves the problem. I've 
placed the free_pages call in xen_blkbk_map itself, but it could also 
be done in xen_blkbk_map_unmap.

---
commit 879ebb502e2f72279553bb0a85cc885ec492a0c1
Author: Roger Pau Monne <roger.pau@xxxxxxxxxx>
Date:   Mon Sep 15 11:01:40 2014 +0200

    xen-blkback: fix leak on grant map error path
    
    Fix leaking a page when a grant mapping has failed.
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reported by: Tao Chen <boby.chen@xxxxxxxxxx>
    ---
    This patch should be backported to stable branches.

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index 64c60ed..63fc7f0 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -763,6 +763,7 @@ again:
                        BUG_ON(new_map_idx >= segs_to_map);
                        if (unlikely(map[new_map_idx].status != 0)) {
                                pr_debug(DRV_PFX "invalid buffer -- could not 
remap it\n");
+                               put_free_pages(blkif, &pages[seg_idx]->page, 1);
                                pages[seg_idx]->handle = BLKBACK_INVALID_HANDLE;
                                ret |= 1;
                                goto next;




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel