Re: A memory leak problem in xen-blkback module

["Roger Pau Monné" <roger.pau@citrix.com>]

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@citrix.com>
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@citrix.com>
    Reported by: Tao Chen <boby.chen@huawei.com>
    ---
    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;