2002-10-13 20:07:19

by Brian Gerst

[permalink] [raw]
Subject: [PATCH] i386 __verify_write fixes

diff -ruN linux-2.5.42/arch/i386/kernel/i386_ksyms.c linux/arch/i386/kernel/i386_ksyms.c
--- linux-2.5.42/arch/i386/kernel/i386_ksyms.c Fri Oct 11 22:51:08 2002
+++ linux/arch/i386/kernel/i386_ksyms.c Sat Oct 12 10:42:36 2002
@@ -69,7 +69,9 @@
#ifdef CONFIG_X86_NUMAQ
EXPORT_SYMBOL(xquad_portio);
#endif
+#ifndef CONFIG_X86_WP_WORKS_OK
EXPORT_SYMBOL(__verify_write);
+#endif
EXPORT_SYMBOL(dump_thread);
EXPORT_SYMBOL(dump_fpu);
EXPORT_SYMBOL(dump_extended_fpu);
diff -ruN linux-2.5.42/arch/i386/mm/fault.c linux/arch/i386/mm/fault.c
--- linux-2.5.42/arch/i386/mm/fault.c Fri Oct 11 22:51:06 2002
+++ linux/arch/i386/mm/fault.c Sat Oct 12 11:11:03 2002
@@ -30,17 +30,20 @@

extern int console_loglevel;

+#ifndef CONFIG_X86_WP_WORKS_OK
/*
* Ugly, ugly, but the goto's result in better assembly..
*/
int __verify_write(const void * addr, unsigned long size)
{
+ struct mm_struct *mm = current->mm;
struct vm_area_struct * vma;
unsigned long start = (unsigned long) addr;

- if (!size)
+ if (!size || segment_eq(get_fs(),KERNEL_DS))
return 1;

+ down_read(&mm->mmap_sem);
vma = find_vma(current->mm, start);
if (!vma)
goto bad_area;
@@ -80,6 +83,11 @@
if (!(vma->vm_flags & VM_WRITE))
goto bad_area;;
}
+ /* FIXME:
+ * We really need to hold mmap_sem over the whole access to
+ * userspace, else another thread could change permissions.
+ */
+ up_read(&mm->mmap_sem);
return 1;

check_stack:
@@ -89,6 +97,7 @@
goto good_area;

bad_area:
+ up_read(&mm->mmap_sem);
return 0;

out_of_memory:
@@ -98,6 +107,7 @@
}
goto bad_area;
}
+#endif

/*
* Unlock any spinlocks which will prevent us from getting the
diff -ruN linux-2.5.42/arch/i386/mm/init.c linux/arch/i386/mm/init.c
--- linux-2.5.42/arch/i386/mm/init.c Sat Oct 12 00:44:41 2002
+++ linux/arch/i386/mm/init.c Sat Oct 12 10:17:21 2002
@@ -378,15 +378,10 @@
* This function cannot be __init, since exceptions don't work in that
* section.
*/
-static int do_test_wp_bit(unsigned long vaddr);
+static int do_test_wp_bit(void);

void __init test_wp_bit(void)
{
- const unsigned long vaddr = PAGE_OFFSET;
- pgd_t *pgd;
- pmd_t *pmd;
- pte_t *pte, old_pte;
-
if (cpu_has_pse) {
/* Ok, all PSE-capable CPUs are definitely handling the WP bit right. */
boot_cpu_data.wp_works_ok = 1;
@@ -395,17 +390,10 @@

printk("Checking if this processor honours the WP bit even in supervisor mode... ");

- pgd = swapper_pg_dir + __pgd_offset(vaddr);
- pmd = pmd_offset(pgd, vaddr);
- pte = pte_offset_kernel(pmd, vaddr);
- old_pte = *pte;
- *pte = pfn_pte(0, PAGE_READONLY);
- local_flush_tlb();
-
- boot_cpu_data.wp_works_ok = do_test_wp_bit(vaddr);
-
- *pte = old_pte;
- local_flush_tlb();
+ /* Any page-aligned address will do, the test is non-destructive */
+ __set_fixmap(FIX_WP_TEST, __pa(&swapper_pg_dir), PAGE_READONLY);
+ boot_cpu_data.wp_works_ok = do_test_wp_bit();
+ clear_fixmap(FIX_WP_TEST);

if (!boot_cpu_data.wp_works_ok) {
printk("No.\n");
@@ -550,7 +538,7 @@
#endif

/* Put this after the callers, so that it cannot be inlined */
-static int do_test_wp_bit(unsigned long vaddr)
+static int do_test_wp_bit(void)
{
char tmp_reg;
int flag;
@@ -564,7 +552,7 @@
" .align 4 \n"
" .long 1b,2b \n"
".previous \n"
- :"=m" (*(char *) vaddr),
+ :"=m" (*(char *)fix_to_virt(FIX_WP_TEST)),
"=q" (tmp_reg),
"=r" (flag)
:"2" (1)
diff -ruN linux-2.5.42/arch/i386/mm/ioremap.c linux/arch/i386/mm/ioremap.c
--- linux-2.5.42/arch/i386/mm/ioremap.c Sun Sep 15 22:18:24 2002
+++ linux/arch/i386/mm/ioremap.c Sat Oct 12 10:17:21 2002
@@ -289,7 +289,7 @@

idx = FIX_BTMAP_BEGIN;
while (nrpages > 0) {
- __set_fixmap(idx, 0, __pgprot(0));
+ clear_fixmap(idx);
--idx;
--nrpages;
}
diff -ruN linux-2.5.42/include/asm-i386/fixmap.h linux/include/asm-i386/fixmap.h
--- linux-2.5.42/include/asm-i386/fixmap.h Sat Oct 12 00:44:45 2002
+++ linux/include/asm-i386/fixmap.h Sat Oct 12 10:17:21 2002
@@ -27,7 +27,7 @@
* Here we define all the compile-time 'special' virtual
* addresses. The point is to have a constant address at
* compile time, but to set the physical address only
- * in the boot process. We allocate these special addresses
+ * in the boot process. We allocate these special addresses
* from the end of virtual memory (0xfffff000) backwards.
* Also this lets us do fail-safe vmalloc(), we
* can guarantee that these special addresses and
@@ -41,13 +41,6 @@
* TLB entries of such buffers will not be flushed across
* task switches.
*/
-
-/*
- * on UP currently we will have no trace of the fixmap mechanizm,
- * no page table allocations, etc. This might change in the
- * future, say framebuffers for the console driver(s) could be
- * fix-mapped?
- */
enum fixed_addresses {
#ifdef CONFIG_X86_LOCAL_APIC
FIX_APIC_BASE, /* local (CPU) APIC) -- required for SMP or not */
@@ -81,6 +74,7 @@
#define NR_FIX_BTMAPS 16
FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
FIX_BTMAP_BEGIN = FIX_BTMAP_END + NR_FIX_BTMAPS - 1,
+ FIX_WP_TEST,
__end_of_fixed_addresses
};

@@ -94,6 +88,10 @@
*/
#define set_fixmap_nocache(idx, phys) \
__set_fixmap(idx, phys, PAGE_KERNEL_NOCACHE)
+
+#define clear_fixmap(idx) \
+ __set_fixmap(idx, 0, __pgprot(0))
+
/*
* used by vmalloc.c.
*
diff -ruN linux-2.5.42/include/asm-i386/uaccess.h linux/include/asm-i386/uaccess.h
--- linux-2.5.42/include/asm-i386/uaccess.h Sun Sep 15 22:18:21 2002
+++ linux/include/asm-i386/uaccess.h Sat Oct 12 10:18:15 2002
@@ -55,7 +55,6 @@

#define access_ok(type,addr,size) ( (__range_ok(addr,size) == 0) && \
((type) == VERIFY_READ || boot_cpu_data.wp_works_ok || \
- segment_eq(get_fs(),KERNEL_DS) || \
__verify_write((void *)(addr),(size))))

#endif


Attachments:
verify_write-1 (5.61 kB)

2002-10-22 14:44:05

by Alan

[permalink] [raw]
Subject: Re: [PATCH] i386 __verify_write fixes

On Sun, 2002-10-13 at 21:13, Brian Gerst wrote:
> mark the page read-only. A 386 would continue to write to the page when
> the first thread woke up again. We need to prevent anyone from changing
> the page permissions until we are done writing to userspace. My
> suggestion is to replace access_ok() with begin_user_{read,write}(), and
> add end_user_{read,write}(). Thoughs?

For 2.6 with the time scale as it is I think we just block pre-emption
on 386. Its not a big deal since almost nobody uses 386 any more