* zach@xxxxxxxxxx (zach@xxxxxxxxxx) wrote:
Generally nice cleanup. Couple comments below.
> Index: linux-2.6.13/arch/i386/kernel/ldt.c
> ===================================================================
> --- linux-2.6.13.orig/arch/i386/kernel/ldt.c 2005-08-08 13:50:20.000000000 -0700
> +++ linux-2.6.13/arch/i386/kernel/ldt.c 2005-08-08 13:53:28.000000000 -0700
> @@ -177,7 +177,7 @@
> static int write_ldt(void __user * ptr, unsigned long bytecount, int oldmode)
> {
> struct mm_struct * mm = current->mm;
> - __u32 entry_1, entry_2, *lp;
> + __u32 entry_1, entry_2;
> int error;
> struct user_desc ldt_info;
>
> @@ -205,8 +205,6 @@
> goto out_unlock;
> }
>
> - lp = (__u32 *) ((ldt_info.entry_number << 3) + (char *) mm->context.ldt);
> -
> /* Allow LDTs to be cleared by the user. */
> if (ldt_info.base_addr == 0 && ldt_info.limit == 0) {
> if (oldmode || LDT_empty(&ldt_info)) {
> @@ -223,8 +221,7 @@
>
> /* Install the new entry ... */
> install:
> - *lp = entry_1;
> - *(lp+1) = entry_2;
> + write_ldt_entry(mm->context.ldt, ldt_info.entry_number, entry_1, entry_2);
> error = 0;
>
> out_unlock:
> Index: linux-2.6.13/arch/i386/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.13.orig/arch/i386/kernel/kprobes.c 2005-08-08 13:50:12.000000000 -0700
> +++ linux-2.6.13/arch/i386/kernel/kprobes.c 2005-08-08 14:16:47.000000000 -0700
> @@ -156,17 +156,15 @@
> struct kprobe *p;
> int ret = 0;
> kprobe_opcode_t *addr = NULL;
> - unsigned long *lp;
>
> /* We're in an interrupt, but this is clear and BUG()-safe. */
> preempt_disable();
> /* Check if the application is using LDT entry for its code segment and
> * calculate the address by reading the base address from the LDT entry.
> */
> - if ((regs->xcs & 4) && (current->mm)) {
> - lp = (unsigned long *) ((unsigned long)((regs->xcs >> 3) * 8)
> - + (char *) current->mm->context.ldt);
> - addr = (kprobe_opcode_t *) (get_desc_base(lp) + regs->eip -
> + if (segment_is_ldt(regs->xcs) && (current->mm)) {
> + struct desc_struct *desc = &LDT_DESCRIPTOR(regs->xcs);
> + addr = (kprobe_opcode_t *) (get_desc_base(desc) + regs->eip -
> sizeof(kprobe_opcode_t));
> } else {
> addr = (kprobe_opcode_t *)(regs->eip - sizeof(kprobe_opcode_t));
> Index: linux-2.6.13/arch/i386/math-emu/fpu_system.h
> ===================================================================
> --- linux-2.6.13.orig/arch/i386/math-emu/fpu_system.h 2005-08-08 13:50:12.000000000 -0700
> +++ linux-2.6.13/arch/i386/math-emu/fpu_system.h 2005-08-08 13:53:28.000000000 -0700
> @@ -22,7 +22,6 @@
>
> /* s is always from a cpu register, and the cpu does bounds checking
> * during register load --> no further bounds checks needed */
> -#define LDT_DESCRIPTOR(s) (((struct desc_struct *)current->mm->context.ldt)[(s) >> 3])
> #define SEG_D_SIZE(x) ((x).b & (3 << 21))
> #define SEG_G_BIT(x) ((x).b & (1 << 23))
> #define SEG_GRANULARITY(x) (((x).b & (1 << 23)) ? 4096 : 1)
> Index: linux-2.6.13/arch/i386/mm/fault.c
> ===================================================================
> --- linux-2.6.13.orig/arch/i386/mm/fault.c 2005-08-08 13:50:12.000000000 -0700
> +++ linux-2.6.13/arch/i386/mm/fault.c 2005-08-08 13:59:07.000000000 -0700
> @@ -75,7 +75,8 @@
> {
> unsigned long eip = regs->eip;
> unsigned seg = regs->xcs & 0xffff;
> - u32 seg_ar, seg_limit, base, *desc;
> + u32 seg_ar, seg_limit, base;
> + struct desc_struct *desc;
>
> /* The standard kernel/user address space limit. */
> *eip_limit = (seg & 3) ? USER_DS.seg : KERNEL_DS.seg;
> @@ -101,19 +102,17 @@
> /* Get the GDT/LDT descriptor base.
> When you look for races in this code remember that
> LDT and other horrors are only used in user space. */
> - if (seg & (1<<2)) {
> + if (segment_is_ldt(seg)) {
> /* Must lock the LDT while reading it. */
> down(¤t->mm->context.sem);
> - desc = current->mm->context.ldt;
> - desc = (void *)desc + (seg & ~7);
> + desc = &LDT_DESCRIPTOR(seg);
> } else {
> /* Must disable preemption while reading the GDT. */
> - desc = (u32 *)&per_cpu(cpu_gdt_table, get_cpu());
> - desc = (void *)desc + (seg & ~7);
> + desc = &per_cpu(cpu_gdt_table, get_cpu())[desc_number(seg)];
Heh, glad you left the comment. This one is getting on the edge of
hiding that get_cpu() is needed for preempt disable.
> }
>
> /* Decode the code segment base from the descriptor */
> - base = get_desc_base((unsigned long *)desc);
> + base = get_desc_base(desc);
>
> if (seg & (1<<2)) {
segment_is_ldt()
> up(¤t->mm->context.sem);
> Index: linux-2.6.13/include/asm-i386/desc.h
> ===================================================================
> --- linux-2.6.13.orig/include/asm-i386/desc.h 2005-08-08 13:50:20.000000000 -0700
> +++ linux-2.6.13/include/asm-i386/desc.h 2005-08-08 15:19:05.000000000 -0700
> @@ -14,6 +14,10 @@
>
> #include <asm/mmu.h>
>
> +#define desc_number(_selector) ((_selector) >> 3)
Any compelling reason not to use _s * LDT_ENTRY_SIZE ?
> +#define segment_is_ldt(_selector) ((_selector) & 4)
> +#define LDT_DESCRIPTOR(s) (((struct desc_struct *)current->mm->context.ldt)[desc_number(s)])
> +
> extern struct desc_struct cpu_gdt_table[GDT_ENTRIES];
> DECLARE_PER_CPU(struct desc_struct, cpu_gdt_table[GDT_ENTRIES]);
>
> @@ -96,6 +100,12 @@
> (info)->seg_not_present == 1 && \
> (info)->useable == 0 )
>
> +static inline void write_ldt_entry(struct desc_struct *ldt, int entry, __u32 entry_a, __u32 entry_b)
> +{
> + ldt[entry].a = entry_a;
> + ldt[entry].b = entry_b;
> +}
> +
> #if TLS_SIZE != 24
> # error update this code.
> #endif
> @@ -140,12 +150,12 @@
> put_cpu();
> }
>
> -static inline unsigned long get_desc_base(unsigned long *desc)
> +static inline unsigned long get_desc_base(struct desc_struct *desc)
> {
> unsigned long base;
> - base = ((desc[0] >> 16) & 0x0000ffff) |
> - ((desc[1] << 16) & 0x00ff0000) |
> - (desc[1] & 0xff000000);
> + base = ((desc->a >> 16) & 0x0000ffff) |
> + ((desc->b << 16) & 0x00ff0000) |
> + (desc->b & 0xff000000);
> return base;
> }
