linux-kernel - Re: [PATCH] x86/mm/pti: Move user W+X check into pti

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]

Date:   Thu, 16 Aug 2018 22:42:48 -0400
From:   "David H. Gutteridge" <dhgutteridge@...patico.ca>
To:     Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        "H . Peter Anvin" <hpa@...or.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Juergen Gross <jgross@...e.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Borislav Petkov <bp@...en8.de>, Jiri Kosina <jkosina@...e.cz>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Brian Gerst <brgerst@...il.com>,
        David Laight <David.Laight@...lab.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        Eduardo Valentin <eduval@...zon.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Will Deacon <will.deacon@....com>, aliguori@...zon.com,
        daniel.gruss@...k.tugraz.at, hughd@...gle.com, keescook@...gle.com,
        Andrea Arcangeli <aarcange@...hat.com>,
        Waiman Long <llong@...hat.com>, Pavel Machek <pavel@....cz>,
        jroedel@...e.de
Subject: Re: [PATCH] x86/mm/pti: Move user W+X check into pti_finalize()

On Wed, 2018-08-08 at 13:16 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@...e.de>
> 
> The user page-table gets the updated kernel mappings in
> pti_finalize(), which runs after the RO+X permissions got
> applied to the kernel page-table in mark_readonly().
> 
> But with CONFIG_DEBUG_WX enabled, the user page-table is
> already checked in mark_readonly() for insecure mappings.
> This causes false-positive warnings, because the user
> page-table did not get the updated mappings yet.
> 
> Move the W+X check for the user page-table into
> pti_finalize() after it updated all required mappings.
> 
> Signed-off-by: Joerg Roedel <jroedel@...e.de>
> ---
>  arch/x86/include/asm/pgtable.h | 7 +++++--
>  arch/x86/mm/dump_pagetables.c  | 3 +--
>  arch/x86/mm/pti.c              | 2 ++
>  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable.h
> b/arch/x86/include/asm/pgtable.h
> index e39088cb..a1cb333 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -30,11 +30,14 @@ int __init __early_make_pgtable(unsigned long
> address, pmdval_t pmd);
>  void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd);
>  void ptdump_walk_pgd_level_debugfs(struct seq_file *m, pgd_t *pgd,
> bool user);
>  void ptdump_walk_pgd_level_checkwx(void);
> +void ptdump_walk_user_pgd_level_checkwx(void);
>  
>  #ifdef CONFIG_DEBUG_WX
> -#define debug_checkwx() ptdump_walk_pgd_level_checkwx()
> +#define debug_checkwx()		ptdump_walk_pgd_level_checkwx()
> +#define debug_checkwx_user()	ptdump_walk_user_pgd_level_checkwx()
>  #else
> -#define debug_checkwx() do { } while (0)
> +#define debug_checkwx()		do { } while (0)
> +#define debug_checkwx_user()	do { } while (0)
>  #endif
>  
>  /*
> diff --git a/arch/x86/mm/dump_pagetables.c
> b/arch/x86/mm/dump_pagetables.c
> index ccd92c4..b8ab901 100644
> --- a/arch/x86/mm/dump_pagetables.c
> +++ b/arch/x86/mm/dump_pagetables.c
> @@ -569,7 +569,7 @@ void ptdump_walk_pgd_level_debugfs(struct seq_file
> *m, pgd_t *pgd, bool user)
>  }
>  EXPORT_SYMBOL_GPL(ptdump_walk_pgd_level_debugfs);
>  
> -static void ptdump_walk_user_pgd_level_checkwx(void)
> +void ptdump_walk_user_pgd_level_checkwx(void)
>  {
>  #ifdef CONFIG_PAGE_TABLE_ISOLATION
>  	pgd_t *pgd = INIT_PGD;
> @@ -586,7 +586,6 @@ static void
> ptdump_walk_user_pgd_level_checkwx(void)
>  void ptdump_walk_pgd_level_checkwx(void)
>  {
>  	ptdump_walk_pgd_level_core(NULL, NULL, true, false);
> -	ptdump_walk_user_pgd_level_checkwx();
>  }
>  
>  static int __init pt_dump_init(void)
> diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
> index 69a9d60..026a89a 100644
> --- a/arch/x86/mm/pti.c
> +++ b/arch/x86/mm/pti.c
> @@ -628,4 +628,6 @@ void pti_finalize(void)
>  	 */
>  	pti_clone_entry_text();
>  	pti_clone_kernel_text();
> +
> +	debug_checkwx_user();
>  }

I've tested this in a VM and on an Atom laptop, as usual. No
regressions noted.

(The version I tested was the latter pulled into tip:
[ tglx: Folded !NX supported fix ])

Tested-by: David H. Gutteridge <dhgutteridge@...patico.ca>

Regards,

Dave