[PATCH] mmaper_kern.c fixes [buffer overruns]
* copy_from_user() can fail; ->write() must check its return value. * severe buffer overruns both in ->read() and ->write() - lseek to the end (i.e. to mmapper_size) and if(count + *ppos > mmapper_size) count = count + *ppos - mmapper_size; will do absolutely nothing. Then it will call copy_to_user(buf,_buf[*ppos],count); with obvious results (similar for ->write()). Fixed by turning read to simple_read_from_buffer() and by doing normal limiting of count in ->write(). * gratitious lock_kernel() in ->mmap() - it's useless there. * lots of gratitious includes. Signed-off-by: Al Viro <[EMAIL PROTECTED]> diff -urN RC13-rc7-base/arch/um/drivers/mmapper_kern.c current/arch/um/drivers/mmapper_kern.c --- RC13-rc7-base/arch/um/drivers/mmapper_kern.c2005-06-17 15:48:29.0 -0400 +++ current/arch/um/drivers/mmapper_kern.c 2005-08-27 01:25:20.0 -0400 @@ -9,19 +9,11 @@ * */ -#include -#include -#include -#include +#include #include #include -#include -#include -#include #include #include -#include -#include #include "mem_user.h" #include "user_util.h" @@ -31,35 +23,22 @@ static char *v_buf = NULL; static ssize_t -mmapper_read(struct file *file, char *buf, size_t count, loff_t *ppos) +mmapper_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { - if(*ppos > mmapper_size) - return -EINVAL; - - if(count + *ppos > mmapper_size) - count = count + *ppos - mmapper_size; - - if(count < 0) - return -EINVAL; - - copy_to_user(buf,_buf[*ppos],count); - - return count; + return simple_read_from_buffer(buf, count, ppos, v_buf, mmapper_size); } static ssize_t -mmapper_write(struct file *file, const char *buf, size_t count, loff_t *ppos) +mmapper_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { - if(*ppos > mmapper_size) + if (*ppos > mmapper_size) return -EINVAL; - if(count + *ppos > mmapper_size) - count = count + *ppos - mmapper_size; - - if(count < 0) - return -EINVAL; + if (count > mmapper_size - *ppos) + count = mmapper_size - *ppos; - copy_from_user(_buf[*ppos],buf,count); + if (copy_from_user(_buf[*ppos], buf, count)) + return -EFAULT; return count; } @@ -77,7 +56,6 @@ int ret = -EINVAL; int size; - lock_kernel(); if (vma->vm_pgoff != 0) goto out; @@ -92,7 +70,6 @@ goto out; ret = 0; out: - unlock_kernel(); return ret; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mmaper_kern.c fixes [buffer overruns]
* copy_from_user() can fail; -write() must check its return value. * severe buffer overruns both in -read() and -write() - lseek to the end (i.e. to mmapper_size) and if(count + *ppos mmapper_size) count = count + *ppos - mmapper_size; will do absolutely nothing. Then it will call copy_to_user(buf,v_buf[*ppos],count); with obvious results (similar for -write()). Fixed by turning read to simple_read_from_buffer() and by doing normal limiting of count in -write(). * gratitious lock_kernel() in -mmap() - it's useless there. * lots of gratitious includes. Signed-off-by: Al Viro [EMAIL PROTECTED] diff -urN RC13-rc7-base/arch/um/drivers/mmapper_kern.c current/arch/um/drivers/mmapper_kern.c --- RC13-rc7-base/arch/um/drivers/mmapper_kern.c2005-06-17 15:48:29.0 -0400 +++ current/arch/um/drivers/mmapper_kern.c 2005-08-27 01:25:20.0 -0400 @@ -9,19 +9,11 @@ * */ -#include linux/types.h -#include linux/kdev_t.h -#include linux/time.h -#include linux/devfs_fs_kernel.h +#include linux/init.h #include linux/module.h #include linux/mm.h -#include linux/slab.h -#include linux/init.h -#include linux/smp_lock.h #include linux/miscdevice.h #include asm/uaccess.h -#include asm/irq.h -#include asm/pgtable.h #include mem_user.h #include user_util.h @@ -31,35 +23,22 @@ static char *v_buf = NULL; static ssize_t -mmapper_read(struct file *file, char *buf, size_t count, loff_t *ppos) +mmapper_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { - if(*ppos mmapper_size) - return -EINVAL; - - if(count + *ppos mmapper_size) - count = count + *ppos - mmapper_size; - - if(count 0) - return -EINVAL; - - copy_to_user(buf,v_buf[*ppos],count); - - return count; + return simple_read_from_buffer(buf, count, ppos, v_buf, mmapper_size); } static ssize_t -mmapper_write(struct file *file, const char *buf, size_t count, loff_t *ppos) +mmapper_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { - if(*ppos mmapper_size) + if (*ppos mmapper_size) return -EINVAL; - if(count + *ppos mmapper_size) - count = count + *ppos - mmapper_size; - - if(count 0) - return -EINVAL; + if (count mmapper_size - *ppos) + count = mmapper_size - *ppos; - copy_from_user(v_buf[*ppos],buf,count); + if (copy_from_user(v_buf[*ppos], buf, count)) + return -EFAULT; return count; } @@ -77,7 +56,6 @@ int ret = -EINVAL; int size; - lock_kernel(); if (vma-vm_pgoff != 0) goto out; @@ -92,7 +70,6 @@ goto out; ret = 0; out: - unlock_kernel(); return ret; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/