linux-kernel - Re: [PATCH 3/4] mm: page_table_check: Make it dependent on !DEVMEM

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 May 2023 18:40:31 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Ruihan Li <lrh2000@....edu.cn>, linux-mm@...ck.org
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Pasha Tatashin <pasha.tatashin@...een.com>,
        Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org
Subject: Re: [PATCH 3/4] mm: page_table_check: Make it dependent on !DEVMEM

On 10.05.23 10:55, Ruihan Li wrote:
> The special device /dev/mem enables users to map arbitrary physical
> memory regions into the user space, which can conflict with the double
> mapping detection logic used by the page table check. For instance,
> pages may change their properties (e.g., from anonymous pages to named
> pages) while they are still being mapped in the user space via /dev/mem,
> leading to "corruption" detected by the page table check.
> 
> To address this issue, the PAGE_TABLE_CHECK config option is now
> dependent on !DEVMM. This ensures that the page table check cannot be
> enabled when /dev/mem is used. It should be noted that /dev/mem itself
> is a significant security issue, and its conflict with a hardening
> technique is understandable.
> 
> Cc: <stable@...r.kernel.org> # 5.17
> Signed-off-by: Ruihan Li <lrh2000@....edu.cn>
> ---
>   Documentation/mm/page_table_check.rst | 18 ++++++++++++++++++
>   mm/Kconfig.debug                      |  2 +-
>   2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/mm/page_table_check.rst b/Documentation/mm/page_table_check.rst
> index cfd8f4117..b04f29230 100644
> --- a/Documentation/mm/page_table_check.rst
> +++ b/Documentation/mm/page_table_check.rst
> @@ -52,3 +52,21 @@ Build kernel with:
>   
>   Optionally, build kernel with PAGE_TABLE_CHECK_ENFORCED in order to have page
>   table support without extra kernel parameter.
> +
> +Implementation notes
> +====================
> +
> +We specifically decided not to use VMA information in order to avoid relying on
> +MM states (except for limited "struct page" info). The page table check is a
> +separate from Linux-MM state machine that verifies that the user accessible
> +pages are not falsely shared.
> +
> +As a result, special devices that violate the model cannot live with
> +PAGE_TABLE_CHECK. Currently, /dev/mem is the only known example. Given it
> +allows users to map arbitrary physical memory regions into the userspace, any
> +pages may change their properties (e.g., from anonymous pages to named pages)
> +while they are still being mapped in the userspace via /dev/mem, leading to
> +"corruption" detected by the page table check. Therefore, the PAGE_TABLE_CHECK
> +config option is now dependent on !DEVMEM. It's worth noting that /dev/mem
> +itself is a significant security issue, and its conflict with a hardening
> +technique is understandable.
> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index a925415b4..37f3d5b20 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -97,7 +97,7 @@ config PAGE_OWNER
>   
>   config PAGE_TABLE_CHECK
>   	bool "Check for invalid mappings in user page tables"
> -	depends on ARCH_SUPPORTS_PAGE_TABLE_CHECK
> +	depends on ARCH_SUPPORTS_PAGE_TABLE_CHECK && !DEVMEM
>   	select PAGE_EXTENSION
>   	help
>   	  Check that anonymous page is not being mapped twice with read write

That might disable it in a lot of environments I'm afraid. I wonder if 
we could allow it for STRICT_DEVMEM. Hm ...
-- 
Thanks,

David / dhildenb