Hi Linus, Now that it is clear that I was completely mistaken and (mis)quoted Alan Cox when I said "he thinks there are security implications", I am re-sending the vwrite patch against 2.4.0-test8-pre1. Yes, I remember I promised to rewrite both vread/vwrite (as they both are ugly) but they say in russian "better a little bird (e.g. a sparrow) in hand than a big one (e.g. a swan) in the sky" :) Regards, Tigran PS. While we are at it, I attached some trivial cleanups of mm/memory.c and mm/vmscan.c. ---------- Forwarded message ---------- Date: Thu, 31 Aug 2000 14:48:21 +0100 (BST) From: Alan Cox <[EMAIL PROTECTED]> To: Tigran Aivazian <[EMAIL PROTECTED]> Cc: Chris Evans <[EMAIL PROTECTED]>, Matthew Kirkwood <[EMAIL PROTECTED]>, Andi Kleen <[EMAIL PROTECTED]>, Alan Cox <[EMAIL PROTECTED]>, Arnaldo Carvalho de Melo <[EMAIL PROTECTED]>, Richard Gooch <[EMAIL PROTECTED]>, [EMAIL PROTECTED] Subject: Re: [PATCH] mtrr: s/suser/capable/ > speaking of /dev/kmem, I always wondered why Alan Cox (or someone > else?) objected to adding vwrite() ability to write to vmalloc'd addresses > via /dev/kmem and this discussion is as good as any to ask here :) If it can be done cleanly without races fore 2.4 then I dont see a problem. I also dont see why it is useful however which is my only objection. You can just use the physical address it maps to anyway diff -urN -X dontdiff linux/drivers/char/mem.c vwrite/drivers/char/mem.c --- linux/drivers/char/mem.c Sat Jun 24 12:44:26 2000 +++ vwrite/drivers/char/mem.c Thu Aug 31 15:22:09 2000 @@ -258,25 +258,27 @@ count -= read; } - kbuf = (char *)__get_free_page(GFP_KERNEL); - if (!kbuf) - return -ENOMEM; - while (count > 0) { - int len = count; - - if (len > PAGE_SIZE) - len = PAGE_SIZE; - len = vread(kbuf, (char *)p, len); - if (len && copy_to_user(buf, kbuf, len)) { - free_page((unsigned long)kbuf); - return -EFAULT; + if (count > 0) { + kbuf = (char *)__get_free_page(GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + while (count > 0) { + int len = count; + + if (len > PAGE_SIZE) + len = PAGE_SIZE; + len = vread(kbuf, (char *)p, len); + if (len && copy_to_user(buf, kbuf, len)) { + free_page((unsigned long)kbuf); + return -EFAULT; + } + count -= len; + buf += len; + virtr += len; + p += len; } - count -= len; - buf += len; - virtr += len; - p += len; + free_page((unsigned long)kbuf); } - free_page((unsigned long)kbuf); *ppos = p; return virtr + read; } @@ -288,12 +290,56 @@ size_t count, loff_t *ppos) { unsigned long p = *ppos; + ssize_t write = 0; + ssize_t virtr = 0; + char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */ + + if (p < (unsigned long) high_memory) { + write = count; + if (count > (unsigned long) high_memory - p) + write = (unsigned long) high_memory - p; - if (p >= (unsigned long) high_memory) - return 0; - if (count > (unsigned long) high_memory - p) - count = (unsigned long) high_memory - p; - return do_write_mem(file, (void*)p, p, buf, count, ppos); +#if defined(__sparc__) || defined(__mc68000__) + /* we don't have page 0 mapped on sparc and m68k.. */ + if (p < PAGE_SIZE && write > 0) { + size_t tmp = PAGE_SIZE - p; + if (tmp > write) tmp = write; + buf += tmp; + p += tmp; + write -= tmp; + count -= tmp; + } +#endif + if (copy_from_user((char *)p, buf, write)) + return -EFAULT; + p += write; + buf += write; + count -= write; + } + + if (count > 0) { + kbuf = (char *)__get_free_page(GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + while (count > 0) { + int len = count; + + if (len > PAGE_SIZE) + len = PAGE_SIZE; + if (len && copy_from_user(kbuf, buf, len)) { + free_page((unsigned long)kbuf); + return -EFAULT; + } + len = vwrite(kbuf, (char *)p, len); + count -= len; + buf += len; + virtr += len; + p += len; + } + free_page((unsigned long)kbuf); + } + *ppos = p; + return virtr + write; } #if !defined(__mc68000__) diff -urN -X dontdiff linux/include/linux/vmalloc.h vwrite/include/linux/vmalloc.h --- linux/include/linux/vmalloc.h Tue Jul 11 19:26:51 2000 +++ vwrite/include/linux/vmalloc.h Thu Aug 31 15:22:54 2000 @@ -22,6 +22,7 @@ extern void vfree(void * addr); extern void * __vmalloc (unsigned long size, int gfp_mask, pgprot_t prot); extern long vread(char *buf, char *addr, unsigned long count); +extern long vwrite(char *buf, char *addr, unsigned long count); extern void vmfree_area_pages(unsigned long address, unsigned long size); extern int vmalloc_area_pages(unsigned long address, unsigned long size, int gfp_mask, pgprot_t prot); diff -urN -X dontdiff linux/mm/memory.c vwrite/mm/memory.c --- linux/mm/memory.c Thu Aug 10 06:51:12 2000 +++ vwrite/mm/memory.c Thu Aug 31 15:30:16 2000 @@ -67,7 +67,7 @@ copy_user_highpage(to, from, address); } -mem_map_t * mem_map = NULL; +mem_map_t * mem_map; /* * Note: this doesn't free the actual pages themselves. That diff -urN -X dontdiff linux/mm/vmalloc.c vwrite/mm/vmalloc.c --- linux/mm/vmalloc.c Thu Aug 10 06:51:12 2000 +++ vwrite/mm/vmalloc.c Thu Aug 31 15:21:02 2000 @@ -4,6 +4,7 @@ * Copyright (C) 1993 Linus Torvalds * Support of BIGMEM added by Gerhard Wichert, Siemens AG, July 1999 * SMP-safe vmalloc/vfree/ioremap, Tigran Aivazian <[EMAIL PROTECTED]>, May 2000 + * vwrite for write_kmem, Amit S. Kale <[EMAIL PROTECTED]>, May 2000 */ #include <linux/malloc.h> @@ -263,6 +264,43 @@ if (count == 0) goto finished; *buf = *addr; + buf++; + addr++; + count--; + } while (--n > 0); + } +finished: + read_unlock(&vmlist_lock); + return buf - buf_start; +} + +long vwrite(char *buf, char *addr, unsigned long count) +{ + struct vm_struct *tmp; + char *vaddr, *buf_start = buf; + unsigned long n; + + /* Don't allow overflow */ + if ((unsigned long) addr + count < count) + count = -(unsigned long) addr; + + read_lock(&vmlist_lock); + for (tmp = vmlist; tmp; tmp = tmp->next) { + vaddr = (char *) tmp->addr; + if (addr >= vaddr + tmp->size - PAGE_SIZE) + continue; + while (addr < vaddr) { + if (count == 0) + goto finished; + buf++; + addr++; + count--; + } + n = vaddr + tmp->size - PAGE_SIZE - addr; + do { + if (count == 0) + goto finished; + *addr = *buf; buf++; addr++; count--; diff -urN -X dontdiff linux/mm/vmscan.c vwrite/mm/vmscan.c --- linux/mm/vmscan.c Thu Aug 10 06:51:12 2000 +++ vwrite/mm/vmscan.c Thu Aug 31 15:30:08 2000 @@ -34,11 +34,13 @@ * using a process that no longer actually exists (it might * have died while we slept). */ -static int try_to_swap_out(struct mm_struct * mm, struct vm_area_struct* vma, unsigned long address, pte_t * page_table, int gfp_mask) +static int try_to_swap_out(struct vm_area_struct * vma, unsigned long address, + pte_t * page_table, int gfp_mask) { pte_t pte; swp_entry_t entry; struct page * page; + struct mm_struct * mm = vma->vm_mm; int (*swapout)(struct page *, struct file *); pte = *page_table; @@ -79,7 +81,7 @@ set_pte(page_table, swp_entry_to_pte(entry)); drop_pte: UnlockPage(page); - vma->vm_mm->rss--; + mm->rss--; flush_tlb_page(vma, address); page_cache_release(page); goto out_failed; @@ -144,9 +146,9 @@ struct file *file = vma->vm_file; if (file) get_file(file); pte_clear(page_table); - vma->vm_mm->rss--; + mm->rss--; flush_tlb_page(vma, address); - vmlist_access_unlock(vma->vm_mm); + vmlist_access_unlock(mm); error = swapout(page, file); UnlockPage(page); if (file) fput(file); @@ -175,10 +177,10 @@ add_to_swap_cache(page, entry); /* Put the swap entry into the pte after the page is in swapcache */ - vma->vm_mm->rss--; + mm->rss--; set_pte(page_table, swp_entry_to_pte(entry)); flush_tlb_page(vma, address); - vmlist_access_unlock(vma->vm_mm); + vmlist_access_unlock(mm); /* OK, do a physical asynchronous write to swap. */ rw_swap_page(WRITE, page, 0); @@ -209,10 +211,11 @@ * (C) 1993 Kai Petzke, [EMAIL PROTECTED] */ -static inline int swap_out_pmd(struct mm_struct * mm, struct vm_area_struct * vma, pmd_t *dir, unsigned long address, unsigned long end, int gfp_mask) +static inline int swap_out_pmd(struct vm_area_struct * vma, pmd_t *dir, unsigned long +address, unsigned long end, int gfp_mask) { pte_t * pte; unsigned long pmd_end; + struct mm_struct * mm = vma->vm_mm; if (pmd_none(*dir)) return 0; @@ -230,8 +233,8 @@ do { int result; - vma->vm_mm->swap_address = address + PAGE_SIZE; - result = try_to_swap_out(mm, vma, address, pte, gfp_mask); + mm->swap_address = address + PAGE_SIZE; + result = try_to_swap_out(vma, address, pte, gfp_mask); if (result) return result; if (!mm->swap_cnt) @@ -242,10 +245,11 @@ return 0; } -static inline int swap_out_pgd(struct mm_struct * mm, struct vm_area_struct * vma, pgd_t *dir, unsigned long address, unsigned long end, int gfp_mask) +static inline int swap_out_pgd(struct vm_area_struct * vma, pgd_t *dir, unsigned long +address, unsigned long end, int gfp_mask) { pmd_t * pmd; unsigned long pgd_end; + struct mm_struct * mm = vma->vm_mm; if (pgd_none(*dir)) return 0; @@ -262,7 +266,7 @@ end = pgd_end; do { - int result = swap_out_pmd(mm, vma, pmd, address, end, gfp_mask); + int result = swap_out_pmd(vma, pmd, address, end, gfp_mask); if (result) return result; if (!mm->swap_cnt) @@ -273,22 +277,23 @@ return 0; } -static int swap_out_vma(struct mm_struct * mm, struct vm_area_struct * vma, unsigned long address, int gfp_mask) +static int swap_out_vma(struct vm_area_struct * vma, unsigned long address, int +gfp_mask) { pgd_t *pgdir; unsigned long end; + struct mm_struct * mm = vma->vm_mm; /* Don't swap out areas which are locked down */ if (vma->vm_flags & VM_LOCKED) return 0; - pgdir = pgd_offset(vma->vm_mm, address); + pgdir = pgd_offset(mm, address); end = vma->vm_end; if (address >= end) BUG(); do { - int result = swap_out_pgd(mm, vma, pgdir, address, end, gfp_mask); + int result = swap_out_pgd(vma, pgdir, address, end, gfp_mask); if (result) return result; if (!mm->swap_cnt) @@ -320,7 +325,7 @@ address = vma->vm_start; for (;;) { - int result = swap_out_vma(mm, vma, address, gfp_mask); + int result = swap_out_vma(vma, address, gfp_mask); if (result) return result; vma = vma->vm_next; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/