[PATCH/2.4.26] Avoid kernel data corruption through /dev/kmem

[PATCH/2.4.26] Avoid kernel data corruption through /dev/kmem

[BlaisorBlade]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

I'm sending this fix for /dev/kmem; I already sent a cleanup about this, but 
since you said "cleanups go in 2.6", then I'm sending only the bugfix.

Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

[-- Attachment #2: fix-mem-return.patch --]
[-- Type: text/x-diff, Size: 1363 bytes --]


We need to check if do_write_mem == -EFAULT.
In fact, without that check, we could execute this:

do_write_mem returns -EFAULT;
wrote = -EFAULT;

buf += wrote; //i.e. buf -= EFAULT (14);

... read other data from buf, and write it to kernel memory
(actually on special circumstances, i.e. p < high_memory && 
 p + count > high_memory).

Luckily not at all exploitable (not even in the OpenBSD idea) since
to write on /dev/kmem you must already be root.

---

 linux-2.4.26-paolo/drivers/char/mem.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff -puN drivers/char/mem.c~fix-mem-return drivers/char/mem.c
--- linux-2.4.26/drivers/char/mem.c~fix-mem-return	2004-07-01 15:14:00.275806312 +0200
+++ linux-2.4.26-paolo/drivers/char/mem.c	2004-07-01 15:28:24.604408392 +0200
@@ -287,11 +287,13 @@ static ssize_t write_kmem(struct file * 
 	char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
 
 	if (p < (unsigned long) high_memory) {
-		wrote = count;
+		ssize_t towrite = count;
 		if (count > (unsigned long) high_memory - p)
-			wrote = (unsigned long) high_memory - p;
+			towrite = (unsigned long) high_memory - p;
 
-		wrote = do_write_mem(file, (void*)p, p, buf, wrote, ppos);
+		wrote = do_write_mem(file, (void*)p, p, buf, towrite, ppos);
+		if (wrote != towrite)
+			return wrote;
 
 		p += wrote;
 		buf += wrote;
_

Re: [PATCH/2.4.26] Avoid kernel data corruption through /dev/kmem

[Marcelo Tosatti]

On Thu, Jul 01, 2004 at 04:05:29PM +0200, BlaisorBlade wrote:
> I'm sending this fix for /dev/kmem; I already sent a cleanup about this, but 
> since you said "cleanups go in 2.6", then I'm sending only the bugfix.

Hi Paolo, 

This looks much better for inclusion. But do you actually have a problem with
write to /dev/kmem not returning correct error code?

If you convince me there are good enough reasons we can try this on 2.4.28-pre.

Thanks

> We need to check if do_write_mem == -EFAULT.
> In fact, without that check, we could execute this:
> 
> do_write_mem returns -EFAULT;
> wrote = -EFAULT;
> 
> buf += wrote; //i.e. buf -= EFAULT (14);
> 
> ... read other data from buf, and write it to kernel memory
> (actually on special circumstances, i.e. p < high_memory && 
>  p + count > high_memory).
> 
> Luckily not at all exploitable (not even in the OpenBSD idea) since
> to write on /dev/kmem you must already be root.
> 
> ---
> 
>  linux-2.4.26-paolo/drivers/char/mem.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff -puN drivers/char/mem.c~fix-mem-return drivers/char/mem.c
> --- linux-2.4.26/drivers/char/mem.c~fix-mem-return	2004-07-01 15:14:00.275806312 +0200
> +++ linux-2.4.26-paolo/drivers/char/mem.c	2004-07-01 15:28:24.604408392 +0200
> @@ -287,11 +287,13 @@ static ssize_t write_kmem(struct file * 
>  	char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
>  
>  	if (p < (unsigned long) high_memory) {
> -		wrote = count;
> +		ssize_t towrite = count;
>  		if (count > (unsigned long) high_memory - p)
> -			wrote = (unsigned long) high_memory - p;
> +			towrite = (unsigned long) high_memory - p;
>  
> -		wrote = do_write_mem(file, (void*)p, p, buf, wrote, ppos);
> +		wrote = do_write_mem(file, (void*)p, p, buf, towrite, ppos);
> +		if (wrote != towrite)
> +			return wrote;
>  
>  		p += wrote;
>  		buf += wrote;
> _

Re: [PATCH/2.4.26] Avoid kernel data corruption through /dev/kmem

[BlaisorBlade]

Alle 15:18, venerdì 2 luglio 2004, Marcelo Tosatti ha scritto:
> On Thu, Jul 01, 2004 at 04:05:29PM +0200, BlaisorBlade wrote:

> This looks much better for inclusion. But do you actually have a problem
> with write to /dev/kmem not returning correct error code?
The *other* time I spoke about error code... but it seems you don't have 
noticed the part *below* of my mail, the one where I describe what I complain 
about kernel memory corruption... read below:

> If you convince me there are good enough reasons we can try this on
> 2.4.28-pre.

> > We need to check if do_write_mem == -EFAULT.
> > In fact, without that check, we could execute this:
> >
> > do_write_mem returns -EFAULT;
> > wrote = -EFAULT;
> >
> > buf += wrote; //i.e. buf -= EFAULT (14);
> >
> > ... read other data from buf, and write it to kernel memory
And those datas are crap, obviously: the app wanted to write *buf to kernel 
memory, not *(buf-14), so we can have data corruption.

> > (actually on special circumstances, i.e. p < high_memory &&
> >  p + count > high_memory).

-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729