linux-kernel - Re: [PATCH] x86/mm: Avoid paravirtualization calls in native_set

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 2 Mar 2018 13:16:59 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
cc:     Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Fengguang Wu <fengguang.wu@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/mm: Avoid paravirtualization calls in
 native_set_p4d()

On Wed, 28 Feb 2018, Kirill A. Shutemov wrote:

> p4d_val() and pgd_val() are paravirtualized and we should avoid using
> them in native_set_p4d().

I'm really unhappy with your changelogs.

... and we should avoid ....

Should is not strong enough as it leaves the option to not do it.

Aside of that you fail (again) to explain the WHY. Something like this:

native_set_p4d() uses p4d_val() and pgd_val() to do ".....". Both functions
are paravirtualized which is wrong because it brings paravirtualization
into the native implementation resulting in "$problem".

> Let's replace them with native_p4d_val() and native_pgd_val().

Let's? Just say:

Replace them with native_p4d_val() and native_pgd_val().

Documentation/process/... explains it really well change logs have to be
written in imperative mood.

Thanks,

	tglx

> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Fengguang Wu <fengguang.wu@...el.com>
> Fixes: 91f606a8fa68 ("x86/mm: Replace compile-time checks for 5-level paging with runtime-time checks")
> ---
>  arch/x86/include/asm/pgtable_64.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 81dda8d1d0bd..163e01a0631d 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -224,9 +224,9 @@ static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
>  		return;
>  	}
>  
> -	pgd = native_make_pgd(p4d_val(p4d));
> +	pgd = native_make_pgd(native_p4d_val(p4d));
>  	pgd = pti_set_user_pgd((pgd_t *)p4dp, pgd);
> -	*p4dp = native_make_p4d(pgd_val(pgd));
> +	*p4dp = native_make_p4d(native_pgd_val(pgd));
>  }
>  
>  static inline void native_p4d_clear(p4d_t *p4d)
> -- 
> 2.16.1
> 
>