| 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
| ||
|
Message-ID: <47584C56.3070804@goop.org>
Date: Thu, 06 Dec 2007 11:24:06 -0800
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Glauber de Oliveira Costa <gcosta@...hat.com>
CC: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
glommer@...il.com, tglx@...utronix.de, mingo@...e.hu,
ehabkost@...hat.com, avi@...ranet.com, anthony@...emonkey.ws,
virtualization@...ts.linux-foundation.org, rusty@...tcorp.com.au,
ak@...e.de, chrisw@...s-sol.org, rostedt@...dmis.org,
hpa@...or.com, zach@...are.com
Subject: Re: [PATCH 1/19] unify desc_struct
Glauber de Oliveira Costa wrote:
> This patch aims to make the access of struct desc_struct variables
> equal across architectures. In this patch, I unify the i386 and x86_64
> versions under an anonymous union, keeping the way they are accessed
> untouched (a and b for 32-bit code, individual bit-fields for 64-bit).
>
> This solution is not beautiful, but will allow us to integrate common
> code that differed by the way descriptors were used. This is to be viewed
> incrementally. There's simply too much code to be fixed at once.
>
> In the future, goal is to set up in a single way of acessing
> the desc_struct fields.
>
> Signed-off-by: Glauber de Oliveira Costa <gcosta@...hat.com>
> ---
> arch/x86/kernel/apm_32.c | 2 +-
> arch/x86/kernel/cpu/common.c | 28 ++++++++++++++--------------
> arch/x86/kernel/process_64.c | 2 +-
> arch/x86/kernel/traps_32.c | 2 +-
> include/asm-x86/desc_defs.h | 28 ++++++++++++++++++++--------
> include/asm-x86/processor_32.h | 5 +----
> 6 files changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index 8cd9778..b8b9339 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -405,7 +405,7 @@ static DECLARE_WAIT_QUEUE_HEAD(apm_waitqueue);
> static DECLARE_WAIT_QUEUE_HEAD(apm_suspend_waitqueue);
> static struct apm_user * user_list;
> static DEFINE_SPINLOCK(user_list_lock);
> -static const struct desc_struct bad_bios_desc = { 0, 0x00409200 };
> +static const struct desc_struct bad_bios_desc = {{{ 0, 0x00409200 }}};
>
> static const char driver_version[] = "1.16ac"; /* no spaces */
>
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 235cd61..0fe1c1d 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -22,31 +22,31 @@
> #include "cpu.h"
>
> DEFINE_PER_CPU(struct gdt_page, gdt_page) = { .gdt = {
> - [GDT_ENTRY_KERNEL_CS] = { 0x0000ffff, 0x00cf9a00 },
> - [GDT_ENTRY_KERNEL_DS] = { 0x0000ffff, 0x00cf9200 },
> - [GDT_ENTRY_DEFAULT_USER_CS] = { 0x0000ffff, 0x00cffa00 },
> - [GDT_ENTRY_DEFAULT_USER_DS] = { 0x0000ffff, 0x00cff200 },
> + [GDT_ENTRY_KERNEL_CS] = {{{ 0x0000ffff, 0x00cf9a00 }}},
> + [GDT_ENTRY_KERNEL_DS] = {{{ 0x0000ffff, 0x00cf9200 }}},
> + [GDT_ENTRY_DEFAULT_USER_CS] = {{{ 0x0000ffff, 0x00cffa00 }}},
> + [GDT_ENTRY_DEFAULT_USER_DS] = {{{ 0x0000ffff, 0x00cff200 }}},
>
I don't suppose there's some way to make all this more symbolic?
> /*
> * Segments used for calling PnP BIOS have byte granularity.
> * They code segments and data segments have fixed 64k limits,
> * the transfer segment sizes are set at run time.
> */
> - [GDT_ENTRY_PNPBIOS_CS32] = { 0x0000ffff, 0x00409a00 },/* 32-bit code */
> - [GDT_ENTRY_PNPBIOS_CS16] = { 0x0000ffff, 0x00009a00 },/* 16-bit code */
> - [GDT_ENTRY_PNPBIOS_DS] = { 0x0000ffff, 0x00009200 }, /* 16-bit data */
> - [GDT_ENTRY_PNPBIOS_TS1] = { 0x00000000, 0x00009200 },/* 16-bit data */
> - [GDT_ENTRY_PNPBIOS_TS2] = { 0x00000000, 0x00009200 },/* 16-bit data */
> + [GDT_ENTRY_PNPBIOS_CS32] = {{{ 0x0000ffff, 0x00409a00 }}},/* 32-bit code */
> + [GDT_ENTRY_PNPBIOS_CS16] = {{{ 0x0000ffff, 0x00009a00 }}},/* 16-bit code */
> + [GDT_ENTRY_PNPBIOS_DS] = {{{ 0x0000ffff, 0x00009200 }}}, /* 16-bit data */
> + [GDT_ENTRY_PNPBIOS_TS1] = {{{ 0x00000000, 0x00009200 }}},/* 16-bit data */
> + [GDT_ENTRY_PNPBIOS_TS2] = {{{ 0x00000000, 0x00009200 }}},/* 16-bit data */
> /*
> * The APM segments have byte granularity and their bases
> * are set at run time. All have 64k limits.
> */
> - [GDT_ENTRY_APMBIOS_BASE] = { 0x0000ffff, 0x00409a00 },/* 32-bit code */
> + [GDT_ENTRY_APMBIOS_BASE] = {{{ 0x0000ffff, 0x00409a00 }}},/* 32-bit code */
> /* 16-bit code */
> - [GDT_ENTRY_APMBIOS_BASE+1] = { 0x0000ffff, 0x00009a00 },
> - [GDT_ENTRY_APMBIOS_BASE+2] = { 0x0000ffff, 0x00409200 }, /* data */
> + [GDT_ENTRY_APMBIOS_BASE+1] = {{{ 0x0000ffff, 0x00009a00 }}},
> + [GDT_ENTRY_APMBIOS_BASE+2] = {{{ 0x0000ffff, 0x00409200 }}}, /* data */
>
> - [GDT_ENTRY_ESPFIX_SS] = { 0x00000000, 0x00c09200 },
> - [GDT_ENTRY_PERCPU] = { 0x00000000, 0x00000000 },
> + [GDT_ENTRY_ESPFIX_SS] = {{{ 0x00000000, 0x00c09200 }}},
> + [GDT_ENTRY_PERCPU] = {{{ 0x00000000, 0x00000000 }}},
> } };
> EXPORT_PER_CPU_SYMBOL_GPL(gdt_page);
>
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index 6724840..9e99cb7 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -437,7 +437,7 @@ static inline void set_32bit_tls(struct task_struct *t, int tls, u32 addr)
> .limit_in_pages = 1,
> .useable = 1,
> };
> - struct n_desc_struct *desc = (void *)t->thread.tls_array;
> + struct desc_struct *desc = (void *)t->thread.tls_array;
> desc += tls;
> desc->a = LDT_entry_a(&ud);
> desc->b = LDT_entry_b(&ud);
> diff --git a/arch/x86/kernel/traps_32.c b/arch/x86/kernel/traps_32.c
> index e15014e..94c5aea 100644
> --- a/arch/x86/kernel/traps_32.c
> +++ b/arch/x86/kernel/traps_32.c
> @@ -76,7 +76,7 @@ char ignore_fpu_irq = 0;
> * F0 0F bug workaround.. We have a special link segment
> * for this.
> */
> -struct desc_struct idt_table[256] __attribute__((__section__(".data.idt"))) = { {0, 0}, };
> +struct desc_struct idt_table[256] __attribute__((__section__(".data.idt"))) = { {{{ 0, 0 }}}, };
>
> asmlinkage void divide_error(void);
> asmlinkage void debug(void);
> diff --git a/include/asm-x86/desc_defs.h b/include/asm-x86/desc_defs.h
> index 0890040..b3db064 100644
> --- a/include/asm-x86/desc_defs.h
> +++ b/include/asm-x86/desc_defs.h
> @@ -11,18 +11,30 @@
>
> #include <linux/types.h>
>
> +/*
> + * FIXME: Acessing the desc_struct through its fields is more elegant,
> + * and should be the one valid thing to do. However, a lot of open code
> + * still touches the a and b acessors, and doing this allow us to do it
> + * incrementally. We keep the signature as a struct, rather than an union,
> + * so we can get rid of it transparently in the future -- glommer
> + */
> +#define raw_desc_struct struct { unsigned int a, b; }
> +#define detailed_desc_struct \
> + struct { \
> + u16 limit0; \
> + u16 base0; \
> + unsigned base1 : 8, type : 4, s : 1, dpl : 2, p : 1; \
> + unsigned limit : 4, avl : 1, l : 1, d : 1, g : 1, base2 :8;\
> + }
> +
> // 8 byte segment descriptor
> struct desc_struct {
> - u16 limit0;
> - u16 base0;
> - unsigned base1 : 8, type : 4, s : 1, dpl : 2, p : 1;
> - unsigned limit : 4, avl : 1, l : 1, d : 1, g : 1, base2 : 8;
> + union {
> + raw_desc_struct;
> + detailed_desc_struct;
> + };
> } __attribute__((packed));
>
Is it really necessary to use #defines? Couldn't you just define the
structures inline, or give them names?
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists