Hello,
this patch adds a few (not so) paranoid checks: most importantly, it makes
sure that the guest physical memory is actually unmapped from user space
before it is freed. (This was probably the source for sporadic crashes
I was experiencing ...)
Also, as the use of the PG_reserved bit appears to be OK (the 'bttv' driver
and other drivers in 2.3.x do it as well), I've replaced the HACK warning
by a comment explaining what it is supposed to do ...
Bye,
Ulrich
ChangeLog entry:
[host-linux.c] [host-linux.h]
Added check to prevent the guest physical memory from being
freed while still mapped to user space.
Added check to prevent duplicate allocation (memory leak!).
Use 'mem_map_(un)reserve' macros instead of manipulating page
flags directly.
[user.c]
Unmap guest physical memory before freeing it.
Patch:
diff -ur fmw-19990825a/kernel/host-linux.c fmw-uw/kernel/host-linux.c
--- fmw-19990825a/kernel/host-linux.c Wed Aug 25 20:54:46 1999
+++ fmw-uw/kernel/host-linux.c Fri Aug 27 00:28:08 1999
@@ -210,6 +210,7 @@
// free the virtual memory
unalloc_vm_pages();
+ mon_ok = 0;
#if LINUX_VERSION_CODE >= VERSION_CODE(2,1,31)
return(0);
@@ -258,6 +259,12 @@
case FMWALLOCVPHYS:
printk(KERN_WARNING "freemware: allocating %luM of memory\n", arg);
+ // Don't allow duplicate allocation
+ if (mon_ok) {
+ printk(KERN_WARNING "freemware: device already in use\n", arg);
+ return (-EBUSY);
+ }
+
// Check that the amount of memory we allocate is reasonable
if (arg > FMW_MAX_PHY_MEGS) {
printk(KERN_WARNING "freemware: request of too much phymem
(%luMegs)\n", arg);
@@ -372,6 +379,13 @@
break;
case FMWTEARDOWN: // tear down VM environment
+
+ // Do *not* free pages that are still mapped to user space!
+ if (inode->i_mmap != NULL) {
+ printk(KERN_WARNING "freemware: guest memory is still mapped!\n");
+ return -EBUSY;
+ }
+
unalloc_vm_pages();
mon_ok = 0;
return(0);
@@ -393,6 +407,12 @@
{
int i, firstpage, nr_pages;
+ /* Must have memory allocated */
+ if (!mon_ok) {
+ printk(KERN_WARNING "freemware: device not initialized\n");
+ return -EACCES;
+ }
+
/* Private mappings make no sense ... */
if ( !(vma->vm_flags & VM_SHARED) ) {
printk(KERN_WARNING "freemware: private mapping\n");
@@ -523,7 +543,10 @@
// give back pages which were allocated for the VM environment
for (p=0; p<vm_pages.guest_n_pages; p++) {
if (vm_pages.guest[p]) {
- clear_bit(PG_reserved, &mem_map[MAP_NR(vm_pages.guest[p])].flags); /*
HACK!!! */
+
+ /* Remove the PG_reserved flag before returning the page */
+ mem_map_unreserve(MAP_NR(vm_pages.guest[p]));
+
free_page(vm_pages.guest[p]);
}
}
@@ -564,11 +587,17 @@
unalloc_vm_pages();
return (-ENOMEM);
}
- set_bit(PG_reserved, &mem_map[MAP_NR(vm_pages.guest[p])].flags); /* HACK!!!
*/
- //printk(KERN_WARNING "freemware: free page laddr was %08x\n",
- // vm_pages.guest[p]);
- //printk(KERN_WARNING "freemware: free page paddr was %08lx\n",
- // virt_to_phys((void *) vm_pages.guest[p]));
+
+ /*
+ * As we want to map these pages to user space, we need to mark
+ * them as 'reserved' pages by setting the PG_reserved bit.
+ *
+ * This has the effect that:
+ * - remap_page_range accepts them as candidates for remapping
+ * - the swapper does *not* try to swap these pages out, even
+ * after they are mapped to user space
+ */
+ mem_map_reserve(MAP_NR(vm_pages.guest[p]));
}
vm_pages.monitor_page_dir = get_free_page(GFP_KERNEL);
diff -ur fmw-19990825a/kernel/include/host-linux.h fmw-uw/kernel/include/host-linux.h
--- fmw-19990825a/kernel/include/host-linux.h Sun Aug 22 18:55:07 1999
+++ fmw-uw/kernel/include/host-linux.h Thu Aug 26 23:20:15 1999
@@ -35,6 +35,7 @@
#include <linux/fs.h>
#include <linux/mm.h>
#include <linux/proc_fs.h>
+#include <linux/wrapper.h>
#include <linux/version.h>
#include <asm/irq.h>
diff -ur fmw-19990825a/user/user.c fmw-uw/user/user.c
--- fmw-19990825a/user/user.c Wed Aug 25 20:54:46 1999
+++ fmw-uw/user/user.c Fri Aug 27 00:29:02 1999
@@ -102,6 +102,12 @@
fprintf(stderr, "Guest counter: %d\n", *(int *)(ptr + COUNTER_OFF));
+ fprintf(stderr, "Unmapping guest physical memory\n");
+ if (munmap(ptr, 8*1024*1024) != 0) {
+ perror("munmap");
+ exit(1);
+ }
+
fprintf(stderr, "Tearing down VM\n");
request = FMWTEARDOWN;
data = 0;
--
Ulrich Weigand,
IMMD 1, Universitaet Erlangen-Nuernberg,
Martensstr. 3, D-91058 Erlangen, Phone: +49 9131 85-7688
