lkml.org 
[lkml]   [2018]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET
From
Date
On Wed, 2018-07-11 at 12:20 +0200, Ingo Molnar wrote:
> * Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> >
> > Add PTRACE interface for CET MSRs.
> Please *always* describe new ABIs in the changelog, in a precise,
> well-documented 
> way.

Ok!

> >
> > 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?

In x86_64_regsets[], there is no [REGSET_TLS].  The core dump code
cannot handle holes in the array.

>
> >
> >  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.)
>

I will fix it.

>
> >
> > --- 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?
>

I don't know if I can change that, will find out.

Thanks,
Yu-cheng

\
 
 \ /
  Last update: 2018-07-15 22:05    [W:0.103 / U:20.260 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site