linux-kernel - Re: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET

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, 11 Jul 2018 12:20:35 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc:     x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-mm@...ck.org,
        linux-arch@...r.kernel.org, linux-api@...r.kernel.org,
        Arnd Bergmann <arnd@...db.de>,
        Andy Lutomirski <luto@...capital.net>,
        Balbir Singh <bsingharora@...il.com>,
        Cyrill Gorcunov <gorcunov@...il.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Florian Weimer <fweimer@...hat.com>,
        "H.J. Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        Kees Cook <keescook@...omiun.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
        Peter Zijlstra <peterz@...radead.org>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Vedvyas Shanbhogue <vedvyas.shanbhogue@...el.com>
Subject: Re: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET


* Yu-cheng Yu <yu-cheng.yu@...el.com> wrote:

> Add PTRACE interface for CET MSRs.

Please *always* describe new ABIs in the changelog, in a precise, well-documented 
way.

> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index e2ee403865eb..ac2bc3a18427 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -49,7 +49,9 @@ enum x86_regset {
>  	REGSET_IOPERM64 = REGSET_XFP,
>  	REGSET_XSTATE,
>  	REGSET_TLS,
> +	REGSET_CET64 = REGSET_TLS,
>  	REGSET_IOPERM32,
> +	REGSET_CET32,
>  };

Why does REGSET_CET64 alias on REGSET_TLS?

>  struct pt_regs_offset {
> @@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
>  		.size = sizeof(long), .align = sizeof(long),
>  		.active = ioperm_active, .get = ioperm_get
>  	},
> +	[REGSET_CET64] = {
> +		.core_note_type = NT_X86_CET,
> +		.n = sizeof(struct cet_user_state) / sizeof(u64),
> +		.size = sizeof(u64), .align = sizeof(u64),
> +		.active = cetregs_active, .get = cetregs_get,
> +		.set = cetregs_set
> +	},

Ok, could we first please make this part of the regset code more readable and 
start the series with a standalone clean-up patch that changes these initializers 
to something more readable:

	[REGSET_CET64] = {
		.core_note_type	= NT_X86_CET,
		.n		= sizeof(struct cet_user_state) / sizeof(u64),
		.size		= sizeof(u64),
		.align		= sizeof(u64),
		.active		= cetregs_active,
		.get		= cetregs_get,
		.set		= cetregs_set
	},

? (I'm demonstrating the cleanup based on REGSET_CET64, but this should be done on 
every other entry first.)


> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -401,6 +401,7 @@ typedef struct elf64_shdr {
>  #define NT_386_TLS	0x200		/* i386 TLS slots (struct user_desc) */
>  #define NT_386_IOPERM	0x201		/* x86 io permission bitmap (1=deny) */
>  #define NT_X86_XSTATE	0x202		/* x86 extended state using xsave */
> +#define NT_X86_CET	0x203		/* x86 cet state */

Acronyms in comments should be in capital letters.

Also, I think I asked this before: why does "Control Flow Enforcement" abbreviate 
to "CET" (which is a well-known acronym for "Central European Time"), not to CFE?

Thanks,

	Ingo