Re: [patch][resend] MAP_HUGETLB munmap fails with size not 2MB aligned

[Hugh Dickins]

On Mon, 30 Mar 2015, KOSAKI Motohiro wrote:
> On Thu, Mar 26, 2015 at 10:08 AM, Eric B Munson <emunson@xxxxxxxxxx> wrote:
> > On 03/26/2015 07:56 AM, Davide Libenzi wrote:
> >> On Wed, 25 Mar 2015, David Rientjes wrote:
> >>
> >>> I looked at this thread at http://marc.info/?t=141392508800001
> >>> since I didn't have it in my mailbox, and I didn't get a chance
> >>> to actually run your test code.
> >>>
> >>> In short, I think what you're saying is that
> >>>
> >>> ptr = mmap(..., 4KB, ..., MAP_HUGETLB | ..., ...) munmap(ptr,
> >>> 4KB) == EINVAL
> >>
> >> I am not sure you have read the email correctly:
> >>
> >> munmap(mmap(size, HUGETLB), size) = EFAIL
> >>
> >> For every size not multiple of the huge page size. Whereas:
> >>
> >> munmap(mmap(size, HUGETLB), ALIGN(size, HUGEPAGE_SIZE)) = OK
> >
> > I think Davide is right here, this is a long existing bug in the
> > MAP_HUGETLB implementation.  Specifically, the mmap man page says:
> >
> > All pages containing a part of the indicated range are unmapped, and
> > subsequent references to these pages will generate SIGSEGV.
> >
> > I realize that huge pages may not have been considered by those that
> > wrote the spec.  But if I read this I would assume that all pages,
> > regardless of size, touched by the munmap() request should be unmapped.
> >
> > Please include
> > Acked-by: Eric B Munson <emunson@xxxxxxxxxx>
> > to the original patch.  I would like to see the mmap man page adjusted
> > to make note of this behavior as well.
> 
> This is just a bug fix and I never think this has large risk. But
> caution, we might revert immediately
> if this patch arise some regression even if it's come from broken
> application code.
> 
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxx>

and, without wishing to be confrontational,
Nacked-by: Hugh Dickins <hughd@xxxxxxxxxx>

I agree with you that the risk on munmap is probably not large;
but I still have no interest in spending more time on changing
twelve-year-old established behaviour in this way, then looking
out for the regressions and preparing to revert.

The risk is larger on mprotect, and the other calls which this
patch as is would leave inconsistent with munmap.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/