Re: [RFC 2/3] mm: page table check

[Jonathan Corbet <corbet@lwn.net>]

Pasha Tatashin <pasha.tatashin@soleen.com> writes:

> Check user page table entries at the time they are added and removed.
>
> Allows to synchronously catch memory corruption issues related to
> double mapping.
>
> When a pte for an anonymous page is added into page table, we verify
> that this pte does not already point to a file backed page, and vice
> versa if this is a file backed page that is being added we verify that
> this page does not have an anonymous mapping
>
> We also enforce that read-only sharing for anonymous pages is allowed
> (i.e. cow after fork). All other sharing must be for file pages.
>
> Page table check allows to protect and debug cases where "struct page"
> metadata became corrupted for some reason. For example, when refcnt or
> mapcount become invalid.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  Documentation/vm/page_table_check.rst |  53 ++++++

Thanks for documenting this feature!  When you add a new RST file,
though, you need to add it to the index.rst file as well so that it is
included in the docs build.

>  MAINTAINERS                           |   9 +
>  arch/Kconfig                          |   3 +
>  include/linux/page_table_check.h      | 147 ++++++++++++++
>  mm/Kconfig.debug                      |  24 +++
>  mm/Makefile                           |   1 +
>  mm/page_alloc.c                       |   4 +
>  mm/page_ext.c                         |   4 +
>  mm/page_table_check.c                 | 264 ++++++++++++++++++++++++++
>  9 files changed, 509 insertions(+)
>  create mode 100644 Documentation/vm/page_table_check.rst
>  create mode 100644 include/linux/page_table_check.h
>  create mode 100644 mm/page_table_check.c
>
> diff --git a/Documentation/vm/page_table_check.rst b/Documentation/vm/page_table_check.rst
> new file mode 100644
> index 000000000000..41435a45869f
> --- /dev/null
> +++ b/Documentation/vm/page_table_check.rst
> @@ -0,0 +1,53 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _page_table_check:

Do you need this label for anything?  As-is it's just added visual
clutter and could come out.

> +================
> +Page Table Check
> +================
> +
> +Page table check allows to hardern the kernel by ensuring that some types of
> +memory corruptions are prevented.
> +
> +Page table check performs extra verifications at the time when new pages become
> +accessible from userspace by getting their page table entries (PTEs PMDs etc.)
> +added into the table.
> +
> +In case of detected corruption, the kernel is crashed. There is a small
> +performance and memory overhead associated with page table check. Thereofre, it
> +is disabled by default but can be optionally enabled on systems where extra
> +hardening outweighs the costs. Also, because page table check is synchronous, it
> +can help with debugging double map memory corruption issues, by crashing kernel
> +at the time wrong mapping occurs instead of later which is often the case with
> +memory corruptions bugs.
> +
> +==============================
> +Double mapping detection logic
> +==============================

I'd use subsection markup (single "==========" line underneath) for the
subsections.

> ++-------------------+-------------------+-------------------+------------------+
> +| Current Mapping   | New mapping       | Permissions       | Rule             |
> ++===================+===================+===================+==================+
> +| Anonymous         | Anonymous         | Read              | Allow            |
> ++-------------------+-------------------+-------------------+------------------+
> +| Anonymous         | Anonymous         | Read / Write      | Prohibit         |
> ++-------------------+-------------------+-------------------+------------------+
> +| Anonymous         | Named             | Any               | Prohibit         |
> ++-------------------+-------------------+-------------------+------------------+
> +| Named             | Anonymous         | Any               | Prohibit         |
> ++-------------------+-------------------+-------------------+------------------+
> +| Named             | Named             | Any               | Allow            |
> ++-------------------+-------------------+-------------------+------------------+
> +
> +=========================
> +Enabling Page Table Check
> +=========================
> +
> +Build kernel with:
> +
> +- PAGE_TABLE_CHECK=y
> +Note, it can only be enabled on platforms where ARCH_SUPPORTS_PAGE_TABLE_CHECK
> +is available.
> +- Boot with 'page_table_check=on' kernel parameter.
> +
> +Optionally, build kernel with PAGE_TABLE_CHECK_ENFORCED in order to have page
> +table support without extra kernel parameter.

Thanks,

jon