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

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

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;
> _