Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write

Rik van Riel
2023-11-03T03:16:17Z

On Thu, 2023-11-02 at 19:37 -0700, Mike Kravetz wrote:
> On 11/02/23 19:24, Mike Kravetz wrote:
> >
> > In the specific case causing the null-ptr-deref, the resv_map
> > pointer
> > (vm_private_data) is NULL.
>
> Hi Rik,
>
> In commit bf4916922c60 hugetlbfs: extend hugetlb_vma_lock to private
> VMAs,
> it correctly says:
>
>     Extend the locking scheme used to protect shared hugetlb mappings
> from
>     truncate vs page fault races, in order to protect private hugetlb
> mappings
>     (with resv_map) against MADV_DONTNEED.
>
> That qualification '(with resv_map)' caught my attention originally,
> and
> I thought about it again while looking into this.  We now cover the
> common
> cases, but there are still quite a few cases where resv_map is NULL
> for
> private mappings.  In such cases, the race between MADV_DONTNEED and
> page
> fault still exists.  Is that a concern?

Honestly, I'm not sure. In hugetlb_dup_vma_private, which is
called at fork time, we have this comment:

* - For MAP_PRIVATE mappings, this is the reserve map which
does
* not apply to children. Faults generated by the children
are
* not guaranteed to succeed, even if read-only.

That suggests we already have no guarantee of faults
succeeding after fork.

>
> With a bit more work we 'could' make sure every hugetlb vma has a
> lock
> to participate in this scheme.
>
> Any thhoughts?

We can certainly close the race between MADV_DONTNEED
and page faults for MAP_PRIVATE mappings in child processes,
but that does not guarantee that we actually have hugetlb
pages for those processes.

In short, I'm not sure :)

--
All Rights Reversed.