[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZiAfB9UI3TYgIXgX@x1n>
Date: Wed, 17 Apr 2024 15:12:07 -0400
From: Peter Xu <peterx@...hat.com>
To: linux-kernel@...r.kernel.org, linux-mm@...ck.org
Cc: Axel Rasmussen <axelrasmussen@...gle.com>,
David Hildenbrand <david@...hat.com>,
Pasha Tatashin <pasha.tatashin@...een.com>,
Nadav Amit <nadav.amit@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2] mm/page_table_check: Support userfault wr-protect
entries
On Wed, Apr 17, 2024 at 02:52:17PM -0400, Peter Xu wrote:
> Allow page_table_check hooks to check over userfaultfd wr-protect criteria
> upon pgtable updates. The rule is no co-existance allowed for any writable
> flag against userfault wr-protect flag.
>
> This should be better than c2da319c2e, where we used to only sanitize such
> issues during a pgtable walk, but when hitting such issue we don't have a
> good chance to know where does that writable bit came from [1], so that
> even the pgtable walk exposes a kernel bug (which is still helpful on
> triaging) but not easy to track and debug.
>
> Now we switch to track the source. It's much easier too with the recent
> introduction of page table check.
>
> There are some limitations with using the page table check here for
> userfaultfd wr-protect purpose:
>
> - It is only enabled with explicit enablement of page table check configs
> and/or boot parameters, but should be good enough to track at least
> syzbot issues, as syzbot should enable PAGE_TABLE_CHECK[_ENFORCED] for
> x86 [1]. We used to have DEBUG_VM but it's now off for most distros,
> while distros also normally not enable PAGE_TABLE_CHECK[_ENFORCED], which
> is similar.
>
> - It conditionally works with the ptep_modify_prot API. It will be
> bypassed when e.g. XEN PV is enabled, however still work for most of the
> rest scenarios, which should be the common cases so should be good
> enough.
>
> - Hugetlb check is a bit hairy, as the page table check cannot identify
> hugetlb pte or normal pte via trapping at set_pte_at(), because of the
> current design where hugetlb maps every layers to pte_t... For example,
> the default set_huge_pte_at() can invoke set_pte_at() directly and lose
> the hugetlb context, treating it the same as a normal pte_t. So far it's
> fine because we have huge_pte_uffd_wp() always equals to pte_uffd_wp() as
> long as supported (x86 only). It'll be a bigger problem when we'll
> define _PAGE_UFFD_WP differently at various pgtable levels, because then
> one huge_pte_uffd_wp() per-arch will stop making sense first.. as of now
> we can leave this for later too.
>
> This patch also removes commit c2da319c2e altogether, as we have something
> better now.
>
> [1] https://lore.kernel.org/all/000000000000dce0530615c89210@google.com/
>
> Cc: Pasha Tatashin <pasha.tatashin@...een.com>
> Signed-off-by: Peter Xu <peterx@...hat.com>
> ---
> v2:
> - Rename __page_table_check_pxx() to page_table_check_pxx_flags(),
> meanwhile move the pte check out of the loop [Pasha]
> - Fix build issues reported from the bot, also added SWP_DEVICE_WRITE which
> was overlooked before
I forgot to update the docs, sorry. I'll send v3.
--
Peter Xu
Powered by blists - more mailing lists