fix writing into /dev/kmem

The patch below fixes a bug in the /dev/kmem driver which causes UML to die if you write to it. Of course, the kernel will die if you write random crap to kernel memory, but at least it doesn't die in a UML-specific way.

This is a cut'n'paste from an xterm, so apply with patch -l.

Jeff

@@ -287,6 +287,7 @@
        ssize_t virtr = 0;
        char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
 
+       p = (unsigned long) __va(p);
        if (p < (unsigned long) high_memory) {
                wrote = count;
                if (count > (unsigned long) high_memory - p)

@@ -321,7 +322,7 @@
                free_page((unsigned long)kbuf);
        }
 
-       *ppos = p;
+       *ppos = __pa((void *) p);
        return virtr + wrote;
 }
 

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Alle 03:36, domenica 29 febbraio 2004, Jeff Dike ha scritto:

This patch is like the one for the read from /dev/kmem (i.e. the one fixing the panic when one does "cat /dev/kmem": you can reproduce this panic on Uml2.6): it makes it behave like /dev/mem (man 4 mem is indeed helpful!). So it is basically not *the right way*.

Also, someone got a panic with the read patch applied while klogd was reading something. The diagnosys was that the /dev/mem driver checks for overflows, while this patched /dev/kmem driver does not. To read the physical address 0x1with /dev/kmem, you are supposed to read 0xc0000001; so with that patch any proper user of /dev/kmem will overflow p.

So, I propose to either return -EFAULT with not accessible addresses (which is the proper fix), or to (at least and as a workaround) correct the "fops" fields of /dev/kmem to redirect it on /dev/mem (at least this avoids the crash better than the current patches).

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Well, if you look at read_kmem, the thing that it reads is physical memory plus kernel virtual memory. I.e. /dev/kmem is /dev/mem + the kernel virtual memory areas.

Addmittedly, I don't see how x86 gets away with dereferencing address 0, but I don't see what's wrong with my fixes. The file offsets being passed in are effectively physical addresses, which need to be converted into virtual addresses before being dereferenced, with is what __va() does, and __pa() undoes.

Also, read_mem does the _va() correctly:

	if (copy_to_user(buf, __va(p), count))
		return -EFAULT;

while read_kmem does not:

	if (copy_to_user(buf, (char *)p, read))
		return -EFAULT;

The only out that I can see is that readers of kmem are supposed to pass in kernel virtual addresses, 0xc0000000 and up. But, if so, those guys fundamentally aren't going to work on UML because those addresses will be wrong.

It would be nice to see exactly what klogd is doing.

0xa0000001, actually, which is what __va does.

Jeff

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Alle 02:31, martedì 2 marzo 2004, Jeff Dike ha scritto:

No, I do not agree (and man 4 mem agrees with me; I've quoted it below). As you later say after reading the code (which contains The Truth), the address passed to /dev/kmem as already been __va'ed. I.e., if those devices were array we could say:

/dev/kmem[ __va (address) ] == /dev/mem [ address ];

Well, physical address 0 does exist, for /dev/mem; /dev/kmem instead returns -EFAULT (try cat /dev/kmem), because copy_to_user returns a fault state. Actually, 0 is derefereced, and the CPU throws an exception; but thanks to the __extable section and "search_exception_table", that exception is "caught". Read Documentation/exception.txt. I.e., copy_to_user actually checks both the "to" and "from" addresses.

Instead, in UML skas mode, possibly because the process VMA's are not mapped in the kernel thread, the page tables for the user are checked by hand (by um_virt_to_phys); but the check is done only for the userspace address (i.e. the "to" address for copy_to_user; everything applies even to copy_from_user, of course).

In tt mode it uses actually exception, even if they are coded by hand without using __extable (I see a flag here, probably set while catching SIGSEGV).

So, probably (to check), the kernel panic would happen only in tt mode.

Anyway, here comes that copy_{to,from}_user are buggy. Since fixing those would have a good performance hit (to test, but that will probably be sensitive; Documentation/exception.txt says exactly this, because most programs have no bug so checking the page tables has a big cost) and klogd would not work anyway, add an XXX comment, add it on your TODO and make /dev/kmem always return -EFAULT, as a workaround; or make it check the address with maybe_map (and #ifdef the change; even other archs have arch-specific code there).

[From below]

Yes, this is the reality.

So, __va(p) will overflow. That has *actually* happened (in fact I studied what I say in these mails when someone got a fault if he loaded the LVM modules). In fact, in the read_mem there is a check against the overflow:

       if (p >= end_mem)
                return 0;

Well, I said you to read mem(4), you didn't, so I quote:

Mem is a character device file that is an image of the main memory of the computer. It may be used, for example, to examine (and even patch) the system.

Byte addresses in mem are interpreted as *physical* memory
addresses.
[...]
The file kmem is the same as mem, except that the kernel virtual memory rather than physical memory is accessed.

In fact, see this:

# cat /dev/kmem
cat: /dev/kmem: Bad address

Well, on *i386* that is 0xc0000001; since klogd is built with i386 kernel headers, it will use those.

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&opÌk
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

This was the stupidity that I was refusing to believe in. This means that if you take a normal klogd and move it to a 2G/2G system, where the kernel starts at 0x80000000, it won't work.

In skas mode, I wasn't expecting any legitimate unfixable kernel mode access faults, so I wasn't setting up the fault_catcher. So, what I did was add a wrapper around the guts of copy_user which does a setjmp, and returns whether it got longjmp-ed back to.

Reading /dev/kmem now produces -EFAULT. Writing to it still causes a hang because write_kmem doesn't check the return value of do_write_mem. With that fixed, everything produces -EFAULT.

The fix will be in the next patch.

Jeff

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Alle 03:02, giovedì 4 marzo 2004, Jeff Dike ha scritto:

Well, that can be stupid (and is), but needing to recompile iptables when upgrading kernel from 2.4.21-Mandrake (i.e. 2.4.21-rc#) to 2.4.21 is even more stupid, right? However that was real.

Yes, fault_catcher (i.e. memory exceptions) works for kernel addresses at very little cost. Not for userspace addresses, sadly.

Well, if you only changed copy_user, there should be some working addresses to read there, right? Even if klogd would need recompilation to handle it right.

In the case /dev/kmem does not work at all we could live with it, but it is a bit unclean.

I'm sending the return check fix to LKML+ Andrew Morton, together with some cleanup for the unused params of do_write_mem.

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id70&alloc_id638&opÌk
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Alle 03:36, domenica 29 febbraio 2004, Jeff Dike ha scritto:

Also, I forgot one thing, about this Oops:

the i386 Oopses, normally, kill only the thread which triggered the fault - while instead Uml panics altogether. Cannot this be changed at arch/um/kernel/trap_kern.c:segv()?

        if(!is_user)

- panic("Kernel mode fault at addr 0x%lx, ip 0x%lx",
- address, ip);
+ die("Oops".....) //some printk calls... can be omitted
+ do_exit(SIGKILL);

like arch/i386/mm/fault.c: do_page_fault() ?
With the above change, cat /dev/kmem could only create an Oops on the cat process (and possibly some deadlock, but better than a straight crash).

By the way: in handle_page_fault(), there is some dead code (attached is the removal patch).

I also do not understand the loop around handle_mm_fault call, but I could be
wrong: are you sure that after a successful handle_mm_fault (i.e. a major or minor fault) pte_present(*pte) could be false? If handle_mm_fault is not successful, we exit the loop anyway.

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

@@ -74,13 +74,7 @@
 			err = -ENOMEM;
 			goto out_of_memory;
 		default:
-			if (current->pid == 1) {
-				up_read(&mm->mmap_sem);
-				yield();
-				down_read(&mm->mmap_sem);
-				goto survive;
-			}
-			goto out;
+			BUG();
 		}
 		pte = pte_offset_kernel(pmd, page);
 	} while(!pte_present(*pte));

If there's an unexplained kernel mode access fault, anything could be wrong.

I don't see that it's reasonable to just kill the process when kernel data could be corrupted as far as we know.

I don't know why x86 thinks it can just kill the process.

That shouldn't be dead code. I think it should be attached to the VM_FAULT_OOM case - I copied it from x86, so check there to be sure.

It's possible that the page could have been swapped out between fixing the fault and the return from handle_mm_fault. Way back in history, I had a check for the same fault happening twice in a row, and panicing if it happened. This was when UML's fault handling still needed debugging. To prevent the case I just mentioned from triggering that panic, I added the loop to make sure the page was good before returning to userspace.

Jeff

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel