[PATCH] mmaper_kern.c fixes [buffer overruns]

2005-08-26 Thread Al Viro
* 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]

2005-08-26 Thread Al Viro
* 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/