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

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] [day] [month] [year] [list]
Message-ID: <CA+CK2bCwHeodpyfHqSzqQWFTrJEYyYFeythYoMAD2eYw7BfNKg@mail.gmail.com>
Date:   Wed, 17 Nov 2021 11:47:08 -0500
From:   Pasha Tatashin <pasha.tatashin@...een.com>
To:     Jonathan Corbet <corbet@....net>
Cc:     LKML <linux-kernel@...r.kernel.org>, linux-mm <linux-mm@...ck.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Rientjes <rientjes@...gle.com>,
        Paul Turner <pjt@...gle.com>, weixugc@...gle.com,
        Greg Thelen <gthelen@...gle.com>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Mike Rapoport <rppt@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>, masahiroy@...nel.org,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        frederic@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>
Subject: Re: [RFC 2/3] mm: page table check

> >  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@....net> wrote:
>
> Pasha Tatashin <pasha.tatashin@...een.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@...een.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