Re: [PATCH] hugetlbfs: fix memory leak for resv_map

[Mike Kravetz <mike.kravetz@oracle.com>]

Thank you for finding this issue.

On 3/2/19 2:47 AM, Yufen Yu wrote:
> When .mknod create a block device file in hugetlbfs, it will
> allocate an inode, and kmalloc a 'struct resv_map' in resv_map_alloc().
> For now, inode->i_mapping->private_data is used to point the resv_map.
> However, when open the device, bd_acquire() will set i_mapping as
> bd_inode->imapping, result in resv_map memory leak.

We are certainly leaking the resv_map.

> We fix the leak by adding a new entry resv_map in hugetlbfs_inode_info.
> It can store resv_map pointer.

This approach preserves the way the existing code always allocates a
resv_map at inode allocation time.  However, it does add an extra word
to every hugetlbfs inode.  My first thought was, why not special case
the block/char inode creation to not allocate a resv_map?  After all,
it is not used in this case.  In fact, we only need/use the resv_map
when mmap'ing a regular file.  It is a waste to allocate the structure
in all other cases.

It seems like we should be able to wait until a call to hugetlb_reserve_pages()
to allocate the inode specific resv_map in much the same way we do for
private mappings.  We could then remove the resv_map allocation at inode
creation time.  Of course, we would still need the code to free the structure
when the inode is destroyed.

I have not looked too closely at this approach, and there may be some
unknown issues.  However, it would address the leak you discovered and
would result in less memory used for hugetlbfs inodes that are never
mmap'ed.

Any thoughts on this approach?

I know it is beyond the scope of your patch.  If you do not want to try this,
I can code up something in a couple days.
-- 
Mike Kravetz