| 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
| ||
|
Date: Tue, 15 Mar 2022 18:01:30 -0500
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>
Cc: dave.hansen@...el.com, len.brown@...el.com, tony.luck@...el.com,
rafael.j.wysocki@...el.com, reinette.chatre@...el.com,
dan.j.williams@...el.com, viro@...iv.linux.org.uk,
keescook@...omium.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] x86: Separate out x86_regset for 32 and 64 bit
Rick Edgecombe <rick.p.edgecombe@...el.com> writes:
> In ptrace, the x86_32_regsets and x86_64_regsets are constructed such that
> there are no gaps in the arrays. This appears to be for two reasons. One,
> the code in fill_thread_core_info() can't handle the gaps. This will be
> addressed in a future patch. And two, not having gaps shrinks the size of
> the array in memory.
>
> Both regset arrays draw their indices from a shared enum x86_regset, but 32
> bit and 64 bit don't all support the same regsets. In the case of
> IA32_EMULATION they can be compiled in at the same time. So this enum has
> to be laid out in a special way such that there are no gaps for both
> x86_32_regsets and x86_64_regsets. This involves creating aliases for
> enum’s that are only in one view or the other, or creating multiple
> versions like in the case of REGSET_IOPERM32/REGSET_IOPERM64.
>
> Simplify the construction of these arrays by just fully separating out the
> enums for 32 bit and 64 bit. Add some bitsize-free defines for
> REGSET_GENERAL and REGSET_FP since they are the only two referred to in
> bitsize generic code.
>
> This should have no functional change and is only changing how constants
> are generated and named. The enum is local to this file, so it does not
> introduce any burden on code calling from other places in the kernel now
> having to worry about whether to use a 32 bit or 64 bit enum name.
>
> [1] https://lore.kernel.org/lkml/20180717162502.32274-1-yu-cheng.yu@intel.com/
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> ---
> arch/x86/kernel/ptrace.c | 60 ++++++++++++++++++++++++++--------------
> 1 file changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index 8d2f2f995539..7a4988d13c43 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -45,16 +45,34 @@
>
> #include "tls.h"
>
> -enum x86_regset {
> - REGSET_GENERAL,
> - REGSET_FP,
> - REGSET_XFP,
> - REGSET_IOPERM64 = REGSET_XFP,
> - REGSET_XSTATE,
> - REGSET_TLS,
> +enum x86_regset_32 {
> + REGSET_GENERAL32,
> + REGSET_FP32,
> + REGSET_XFP32,
> + REGSET_XSTATE32,
> + REGSET_TLS32,
> REGSET_IOPERM32,
> };
>
> +enum x86_regset_64 {
> + REGSET_GENERAL64,
> + REGSET_FP64,
> + REGSET_IOPERM64,
> + REGSET_XSTATE64,
> +};
So I am looking at this and am wondering if the enums should be:
enum x86_32_regset {
REGSET32_GENERAL,
REGSET32_FP,
REGSET32_XFP,
REGSET32_XSTATE,
REGSET32_TLS,
REGSET32_IOPERM32,
};
enum x86_64_regset {
REGSET64_GENERAL,
REGSET64_FP,
REGSET64_IOPERM64,
REGSET64_XSTATE,
};
That is named in such a way that it emphasizes that the difference is
the architecture. Otherwise it reads like the difference is the size of
the registers in the regset. I am pretty certain that in your
REGSET_FP32 and REGSET_FP64 all of the registers are 80 bits long.
Eric
> +
> +#define REGSET_GENERAL \
> +({ \
> + BUILD_BUG_ON((int)REGSET_GENERAL32 != (int)REGSET_GENERAL64); \
> + REGSET_GENERAL32; \
> +})
> +
> +#define REGSET_FP \
> + BUILD_BUG_ON((int)REGSET_FP32 != (int)REGSET_FP64); \
> + REGSET_FP32; \
> +})
> +
> struct pt_regs_offset {
> const char *name;
> int offset;
> @@ -789,13 +807,13 @@ long arch_ptrace(struct task_struct *child, long request,
> #ifdef CONFIG_X86_32
> case PTRACE_GETFPXREGS: /* Get the child extended FPU state. */
> return copy_regset_to_user(child, &user_x86_32_view,
> - REGSET_XFP,
> + REGSET_XFP32,
> 0, sizeof(struct user_fxsr_struct),
> datap) ? -EIO : 0;
>
> case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
> return copy_regset_from_user(child, &user_x86_32_view,
> - REGSET_XFP,
> + REGSET_XFP32,
> 0, sizeof(struct user_fxsr_struct),
> datap) ? -EIO : 0;
> #endif
> @@ -1087,13 +1105,13 @@ static long ia32_arch_ptrace(struct task_struct *child, compat_long_t request,
>
> case PTRACE_GETFPXREGS: /* Get the child extended FPU state. */
> return copy_regset_to_user(child, &user_x86_32_view,
> - REGSET_XFP, 0,
> + REGSET_XFP32, 0,
> sizeof(struct user32_fxsr_struct),
> datap);
>
> case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
> return copy_regset_from_user(child, &user_x86_32_view,
> - REGSET_XFP, 0,
> + REGSET_XFP32, 0,
> sizeof(struct user32_fxsr_struct),
> datap);
>
> @@ -1216,19 +1234,19 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> #ifdef CONFIG_X86_64
>
> static struct user_regset x86_64_regsets[] __ro_after_init = {
> - [REGSET_GENERAL] = {
> + [REGSET_GENERAL64] = {
> .core_note_type = NT_PRSTATUS,
> .n = sizeof(struct user_regs_struct) / sizeof(long),
> .size = sizeof(long), .align = sizeof(long),
> .regset_get = genregs_get, .set = genregs_set
> },
> - [REGSET_FP] = {
> + [REGSET_FP64] = {
> .core_note_type = NT_PRFPREG,
> .n = sizeof(struct fxregs_state) / sizeof(long),
> .size = sizeof(long), .align = sizeof(long),
> .active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
> },
> - [REGSET_XSTATE] = {
> + [REGSET_XSTATE64] = {
> .core_note_type = NT_X86_XSTATE,
> .size = sizeof(u64), .align = sizeof(u64),
> .active = xstateregs_active, .regset_get = xstateregs_get,
> @@ -1257,31 +1275,31 @@ static const struct user_regset_view user_x86_64_view = {
>
> #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
> static struct user_regset x86_32_regsets[] __ro_after_init = {
> - [REGSET_GENERAL] = {
> + [REGSET_GENERAL32] = {
> .core_note_type = NT_PRSTATUS,
> .n = sizeof(struct user_regs_struct32) / sizeof(u32),
> .size = sizeof(u32), .align = sizeof(u32),
> .regset_get = genregs32_get, .set = genregs32_set
> },
> - [REGSET_FP] = {
> + [REGSET_FP32] = {
> .core_note_type = NT_PRFPREG,
> .n = sizeof(struct user_i387_ia32_struct) / sizeof(u32),
> .size = sizeof(u32), .align = sizeof(u32),
> .active = regset_fpregs_active, .regset_get = fpregs_get, .set = fpregs_set
> },
> - [REGSET_XFP] = {
> + [REGSET_XFP32] = {
> .core_note_type = NT_PRXFPREG,
> .n = sizeof(struct fxregs_state) / sizeof(u32),
> .size = sizeof(u32), .align = sizeof(u32),
> .active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
> },
> - [REGSET_XSTATE] = {
> + [REGSET_XSTATE32] = {
> .core_note_type = NT_X86_XSTATE,
> .size = sizeof(u64), .align = sizeof(u64),
> .active = xstateregs_active, .regset_get = xstateregs_get,
> .set = xstateregs_set
> },
> - [REGSET_TLS] = {
> + [REGSET_TLS32] = {
> .core_note_type = NT_386_TLS,
> .n = GDT_ENTRY_TLS_ENTRIES, .bias = GDT_ENTRY_TLS_MIN,
> .size = sizeof(struct user_desc),
> @@ -1312,10 +1330,10 @@ u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
> void __init update_regset_xstate_info(unsigned int size, u64 xstate_mask)
> {
> #ifdef CONFIG_X86_64
> - x86_64_regsets[REGSET_XSTATE].n = size / sizeof(u64);
> + x86_64_regsets[REGSET_XSTATE64].n = size / sizeof(u64);
> #endif
> #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
> - x86_32_regsets[REGSET_XSTATE].n = size / sizeof(u64);
> + x86_32_regsets[REGSET_XSTATE32].n = size / sizeof(u64);
> #endif
> xstate_fx_sw_bytes[USER_XSTATE_XCR0_WORD] = xstate_mask;
> }
Powered by blists - more mailing lists