On 4/9/19 9:20 PM, yuyufen wrote:
> 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@xxxxxxxxxxxxxxx>
>>> Cc: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
>>> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
>>> Signed-off-by: Yufen Yu <yuyufen@xxxxxxxxxx>
>> 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.
I do not believe it is possible to hit this condition in the existing code.
Why? hugetlb_reserve_pages is only called from two places:
1) hugetlb_file_setup. In this case the inode is created immediately before
the call with S_IFREG. Hence a regular file so resv_map created.
2) hugetlbfs_file_mmap called via do_mmap. In do_mmap, there is the following
check:
if (!file->f_op->mmap)
return -ENODEV;
In the hugetlbfs inode creation code (hugetlbfs_get_inode), note that
inode->i_fop = &hugetlbfs_file_operations (containing hugetlbfs_file_mmap)
is only set for inodes of type S_IFREG. And, resv_map are created
for these. So, mmap will not call hugetlbfs_file_mmap for non-S_IFREG
hugetlbfs inode. Instead, it will return ENODEV.
Even if we can not hit this condition today, I still believe it would be
a good idea to make this type of change. It would prevent a possible NULL
dereference in case the structure of code changes in the future. However,
unless I am mistaken this is not needed as an urgent fix.
--
Mike Kravetz
