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

[yuyufen <yuyufen@huawei.com>]

Hi, Mike


On 2019/4/10 11:38, Mike Kravetz wrote:
> 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.

I honestly say that I don't have verified.

>> ---
>>   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 ...

Thanks for your suggestion. It is more reasonable to use -EACCES.

Yufen
Thanks.