Re: [uml-devel] [tracked down] Kernel mode fault with /dev/kmem

[BlaisorBlade <blaisorblade_spam@yahoo.it>]

Alle 20:18, mercoledì 7 gennaio 2004, Adam Heath ha scritto:
> On Thu, 8 Jan 2004, Jeff Chua wrote:
> > open("/dev/kmem", O_RDONLY)             = 3
> > _llseek(0x3, 0, 0xa8965000, 0xbffff934, 0) = 0
> > read(3, Kernel panic: Kernel mode fault at addr 0x48965000, ip 0x400df1a4
>
> 0xa8965000 != 0x48965000
>
> Note how only the first octet is different.

Ok, excellent note; or better, that behaviour (a different address) *is* 
right, but the difference is *wrong*.

Reading the code of read_kmem as patched, we get this pseudo-code:

p=*ppos; //position pointer
...
p = __va(p); /*translate physical address to virtual, like: */
//p += uml_physmem;

However, starting from 0xa8965000, and increasing it by 2,5Gb, we get an 
overflow, i.e. p becomes <old p> + 2,5Gb - 4Gb = <old p> - 1,5 Gb=0x48965000.

So we've got to the bug. But anyway, doing p=__va(p) is a wrong fix because an 
offset inside /dev/kmem is already virtual (comments in the file are clear), 
and in read_mem we see:
  copy_to_user(buf, __va(p), count).

So, the patch just turns /dev/kmem into /dev/mem, on reading, while it doesn't 
do the overflow check done inside read_mem. On writing, instead, it changes 
nothing, so we would have probably other panics.

What I've read about the patch was that the kmem driver "incorrectly assumes 
the address space starts from the 0 address", IIRC. If this is correct, then 
the correct fix would be to check whether the address is mapped and to 
provide a suitable error code, either -EFAULT, or 0, as here:

static ssize_t read_mem(struct file * file, char * buf,
                        size_t count, loff_t *ppos) {
        unsigned long p = *ppos;
        unsigned long end_mem;
        ssize_t read;

        end_mem = __pa(high_memory);
        if (p >= end_mem)
                return 0;
	/*...*/
}

However, I'm not up to see how to exactly do the check (even if this check is 
already done when using the copy_from_user macros).

Bye
-- 
cat <<EOSIGN
Paolo Giarrusso, aka Blaisorblade
Linux Kernel 2.4.23/2.6.0 on an i686; Linux registered user n. 292729
EOSIGN


-------------------------------------------------------
This SF.net email is sponsored by: Perforce Software.
Perforce is the Fast Software Configuration Management System offering
advanced branching capabilities and atomic changes on 50+ platforms.
Free Eval! http://www.perforce.com/perforce/loadprog.html
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel