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

[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.

I will add the index.rst changes.

>
> >  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.

Sure, I will remove it

>
> > +================
> > +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.

I will change to subsection.

Thanks,
Pasha

On Wed, Nov 17, 2021 at 3:08 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> 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