linux-kernel - Re: [PATCH] mm,madvise,hugetlb: fix unexpected data loss with MADV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y1MuSFgBkPZmLu7o@monkey>
Date:   Fri, 21 Oct 2022 16:42:00 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Rik van Riel <riel@...riel.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        kernel-team@...a.com, Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>
Subject: Re: [PATCH] mm,madvise,hugetlb: fix unexpected data loss with
 MADV_DONTNEED on hugetlbfs

On 10/21/22 19:29, Rik van Riel wrote:
> On Fri, 2022-10-21 at 13:48 -0700, Mike Kravetz wrote:
> > On 10/21/22 15:45, Rik van Riel wrote:
> > > A common use case for hugetlbfs is for the application to create
> > > memory pools backed by huge pages, which then get handed over to
> > > some malloc library (eg. jemalloc) for further management.
> > > 
> > > That malloc library may be doing MADV_DONTNEED calls on memory
> > > that is no longer needed, expecting those calls to happen on
> > > PAGE_SIZE boundaries.
> > > 
> > 
> > Thanks Rik.  I tend to agree with this direction as it is 'breaking'
> > current code.  David and I discussed this in this thread,
> > https://lore.kernel.org/linux-mm/356a4b9a-1f56-ae06-b211-bd32fc93ecda@redhat.com/
> > 
> > One thing to note is that there was not any documentation saying
> > madvise would happen on page boundaries.  The system call takes a
> > length and rounds up to page size.  However, the man page explicitly
> > said it operates on a byte range.  Certainly mm people and others
> > know we only operate on pages.  But, that is not what was documented.
> > 
> > When the change was made to add hugetlb support, the decision was
> > made
> > to round up the range to hugetlb page boundaries in hugetlb vmas. 
> > This
> > was to be consistent with how madvise operated on base pages.  At the
> > same time, madvise documentation was updated say it operates on page
> > boundaries as well as the behavior for hugetlb mappings.  If moving
> > forward with this change we will need to update the man page.
> 
> I'll send in a patch for the man page after the patch gets
> merged. I'll change the text to clarify that the system
> may round up the specified length to PAGE_SIZE granularity,
> which is a quantity programs can get through (IIRC) getconf.

Ok, the man page now says madvise only operates on page granularity.
Perhaps we can add info about using sysconf(PAGE_SIZE) if that will
help.  What will really need to be changed is the description of
rounding hugetlb mappings up to the next huge page size boundary.
Just need to describe the hugetlb behavior.  Unfortunately, we may
need to define/describe the arithmetic:
	ALIGN_DOWN(PAGE_ALIGN(length))
i.e. Will align down except in the case length is within PAGE_SIZE
     of hugetlb page size.
-- 
Mike Kravetz