linux-kernel - Re: [PATCH 1/2] fs/proc/task_mmu: add guard region bit to pagemap

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cd57ed04-c6b1-4df3-a5cb-a33078a08e74@lucifer.local>
Date: Mon, 24 Feb 2025 10:18:43 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Kalesh Singh <kaleshsingh@...gle.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Matthew Wilcox <willy@...radead.org>, Vlastimil Babka <vbabka@...e.cz>,
        "Paul E . McKenney" <paulmck@...nel.org>, Jann Horn <jannh@...gle.com>,
        Juan Yescas <jyescas@...gle.com>, linux-mm@...ck.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-api@...r.kernel.org
Subject: Re: [PATCH 1/2] fs/proc/task_mmu: add guard region bit to pagemap

On Mon, Feb 24, 2025 at 10:27:28AM +0100, David Hildenbrand wrote:
> On 21.02.25 13:05, Lorenzo Stoakes wrote:
> > Currently there is no means by which users can determine whether a given
> > page in memory is in fact a guard region, that is having had the
> > MADV_GUARD_INSTALL madvise() flag applied to it.
> >
> > This is intentional, as to provide this information in VMA metadata would
> > contradict the intent of the feature (providing a means to change fault
> > behaviour at a page table level rather than a VMA level), and would require
> > VMA metadata operations to scan page tables, which is unacceptable.
> >
> > In many cases, users have no need to reflect and determine what regions
> > have been designated guard regions, as it is the user who has established
> > them in the first place.
> >
> > But in some instances, such as monitoring software, or software that relies
> > upon being able to ascertain the nature of mappings within a remote process
> > for instance, it becomes useful to be able to determine which pages have
> > the guard region marker applied.
> >
> > This patch makes use of an unused pagemap bit (58) to provide this
> > information.
> >
> > This patch updates the documentation at the same time as making the change
> > such that the implementation of the feature and the documentation of it are
> > tied together.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> > ---
>
>
> Acked-by: David Hildenbrand <david@...hat.com>

Thanks! :)
>
> Something that might be interesting is also extending the PAGEMAP_SCAN
> ioctl.

Yeah, funny you should mention that, I did see that, but on reading the man
page it struck me that it requires the region to be uffd afaict? All the
tests seem to establish uffd, and the man page implies it:

       To start tracking the written state (flag) of a page or range of
       memory, the UFFD_FEATURE_WP_ASYNC must be enabled by UFFDIO_API
       ioctl(2) on userfaultfd and memory range must be registered with
       UFFDIO_REGISTER ioctl(2) in UFFDIO_REGISTER_MODE_WP mode.

It would be a bit of a weird edge case to add support there. I was excited
when I first saw this ioctl, then disappointed afterwards... but maybe I
got it wrong?

>
>
> See do_pagemap_scan().
>
> The benefit here might be that one could effectively search/filter for guard
> regions without copying 64bit per base-page to user space.
>
> But the idea would be to indicate something like PAGE_IS_GUARD_REGION as a
> category when we hit a guard region entry in pagemap_page_category().
>
> (the code is a bit complicated, and I am not sure why we indicate
> PAGE_IS_SWAPPED for non-swap entries, likely wrong ...)

Yeah, I could go on here about how much I hate how uffd does a 'parallel
implementation' of a ton of stuff and then chucks in if (uffd) { go do
something weird + wonderful } but I'll resist the urge :P :))

Do you think, if it were uffd-specific, this would be useful?

At any rate, I'm not sure it's _hugely_ beneficial in this form as pagemap
is binary in any case so you're not having to deal with overhead of parsing
a text file at least!

>
> --
> Cheers,
>
> David / dhildenb
>

Thanks!