Re: [PATCH] hugetlbfs: fix protential null pointer dereference

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

On 4/9/19 7:50 PM, Yufen Yu wrote:
> After commit 58b6e5e8f1ad ("hugetlbfs: fix memory leak for resv_map"),
> i_mapping->private_data will be NULL for mode that is not regular and link.
> Then, it might cause NULL pointer derefernce in hugetlb_reserve_pages()
> when do_mmap. We can avoid protential null pointer dereference by
> judging whether it have been allocated.
> 
> Fixes: 58b6e5e8f1ad ("hugetlbfs: fix memory leak for resv_map")
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: Yufen Yu <yuyufen@huawei.com>

Thanks for catching this.  I mistakenly thought all the code was checking
for NULL resv_map.  That certainly is one (and only) place where it is not
checked.  Have you verified that this is possible?  Should be pretty easy
to do.  If you have not, I can try to verify tomorrow.

> ---
>  mm/hugetlb.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 97b1e0290c66..15e4baf2aa7d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4465,6 +4465,8 @@ int hugetlb_reserve_pages(struct inode *inode,
>  	 */
>  	if (!vma || vma->vm_flags & VM_MAYSHARE) {
>  		resv_map = inode_resv_map(inode);
> +		if (!resv_map)
> +			return -EOPNOTSUPP;

I'm not sure about the return code here.  Note that all callers of
hugetlb_reserve_pages() force return value of -ENOMEM if non-zero value
is returned.  I think we would like to return -EACCES in this situation.
The mmap man page says:

       EACCES A  file descriptor refers to a non-regular file.  Or ...
-- 
Mike Kravetz

>  
>  		chg = region_chg(resv_map, from, to);
>  
>