Re: [PATCH] Missing return value check on do_write_mem

[Andrew Morton <akpm@osdl.org>]

BlaisorBlade <blaisorblade_spam@yahoo.it> wrote:
>
> In drivers/char/mem.c do_write_mem can return -EFAULT but write_kmem forgets 
> this and goes blindly.
> 
> Note: /dev/kmem can be written to only by root, so this *cannot* have security 
> implications.
> 
> Also, do_write_mem takes two unused params and is static - so I've removed 
> those. I actually double-checked this - however please test compilation on 
> Sparc/m68k, since there are some #ifdef.
> 

It's a small thing, but:

> --- ./drivers/char/mem.c.fix	2004-02-20 16:27:21.000000000 +0100
> +++ ./drivers/char/mem.c	2004-03-08 12:17:23.000000000 +0100
> @@ -96,13 +96,14 @@
>  }
>  #endif
>  
> -static ssize_t do_write_mem(struct file * file, void *p, unsigned long realp,
> -			    const char * buf, size_t count, loff_t *ppos)
> +static ssize_t do_write_mem(void *p, const char * buf, size_t count,
> +			    loff_t *ppos)
>  {
> -	ssize_t written;
> +	ssize_t written = 0;
>  
> -	written = 0;
>  #if defined(__sparc__) || (defined(__mc68000__) && defined(CONFIG_MMU))
> +	unsigned long realp = *ppos;
> +

A thread which shares this fd can alter the value at *ppos at any time via
lseek() .

>  	/* we don't have page 0 mapped on sparc and m68k.. */
>  	if (realp < PAGE_SIZE) {
>  		unsigned long sz = PAGE_SIZE-realp;
> @@ -165,7 +166,7 @@
>  
>  	if (!valid_phys_addr_range(p, &count))
>  		return -EFAULT;
> -	return do_write_mem(file, __va(p), p, buf, count, ppos);
> +	return do_write_mem(__va(p), buf, count, ppos);
>  }
>  
>  static int mmap_mem(struct file * file, struct vm_area_struct * vma)
> @@ -276,7 +277,9 @@
>  		if (count > (unsigned long) high_memory - p)
>  			wrote = (unsigned long) high_memory - p;

Here we have applied a range check.

> -		wrote = do_write_mem(file, (void*)p, p, buf, wrote, ppos);
> +		wrote = do_write_mem((void*)p, buf, wrote, ppos);

But here we go off and use *ppos, which may now have a different value from
that which we just range-checked.