Received-SPF: pass (google.com: domain of linux-kernel+bounces-149265-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240417212549.2766883-1-peterx@redhat.com>
In-Reply-To: <20240417212549.2766883-1-peterx@redhat.com>
From: Pasha Tatashin <pasha.tatashin@soleen.com>
Date: Wed, 17 Apr 2024 17:46:56 -0400
Message-ID: <CA+CK2bB9x+ErL+Kx+50DL-8gRSYW6vFen+bFe+cgXPKfuwpG3g@mail.gmail.com>
Subject: Re: [PATCH v4] mm/page_table_check: Support userfault wr-protect entries
To: Peter Xu <peterx@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, 
	Nadav Amit <nadav.amit@gmail.com>, Andrew Morton <akpm@linux-foundation.org>, 
	Axel Rasmussen <axelrasmussen@google.com>, David Hildenbrand <david@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 17, 2024 at 5:25=E2=80=AFPM Peter Xu <peterx@redhat.com> wrote:
>
> Allow page_table_check hooks to check over userfaultfd wr-protect criteri=
a
> upon pgtable updates.  The rule is no co-existance allowed for any writab=
le
> flag against userfault wr-protect flag.
>
> This should be better than c2da319c2e, where we used to only sanitize suc=
h
> 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 confi=
gs
>   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], whi=
ch
>   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 th=
e
>   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 the=
n
>   one huge_pte_uffd_wp() per-arch will stop making sense first.. as of no=
w
>   we can leave this for later too.
>
> This patch also removes commit c2da319c2e altogether, as we have somethin=
g
> better now.
>
> [1] https://lore.kernel.org/all/000000000000dce0530615c89210@google.com/
>
> Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
> Signed-off-by: Peter Xu <peterx@redhat.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 whi=
ch
>   was overlooked before
> v3:
> - Add missing doc update [Pasha]
> v4:
> - Fix wordings in doc, use more elegant swap helpers [Pasha]
> ---
>  Documentation/mm/page_table_check.rst |  9 +++++++-
>  arch/x86/include/asm/pgtable.h        | 18 +---------------
>  mm/page_table_check.c                 | 30 +++++++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 18 deletions(-)

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Thanks,
Pasha