[PATCH] make devmem map only a single page at end of memory

Michael Abbott michael at araneidae.co.uk
Tue Apr 13 13:14:37 UTC 2010


On Tue, 13 Apr 2010, Jörgen Pihlflyckt wrote:
> I noticed that the devmem applet maps two consecutive pages of memory in 
> order to allow access to a (non-aligned) object that spans two pages. 
> This is a problem if we are working with the last page of memory because 
> the next page would wrap around to the first page of memory.
> 
> This patch makes devmem map only a single page if it is at the end of 
> the memory address space.

Forgive me (and this patch isn't tested), but if you're going to do this, 
let's do it right:


diff --git a/miscutils/devmem.c b/miscutils/devmem.c
index e13dedc..225a9ed 100644
--- a/miscutils/devmem.c
+++ b/miscutils/devmem.c
@@ -14,6 +14,8 @@ int devmem_main(int argc UNUSED_PARAM, char **argv)
 	uint64_t writeval = writeval; /* for compiler */
 	off_t target;
 	unsigned page_size = getpagesize();
+	off_t page_mask = ~(off_t)(page_size - 1);
+	size_t map_length;
 	int fd;
 	int width = 8 * sizeof(int);
 
@@ -57,18 +59,19 @@ int devmem_main(int argc UNUSED_PARAM, char **argv)
 		bb_show_usage(); /* bb_strtouXX failed */
 
 	fd = xopen("/dev/mem", argv[3] ? (O_RDWR | O_SYNC) : (O_RDONLY | O_SYNC));
+	map_length = ((target + width - 1) & page_mask) - (target & page_mask) + page_size;
 	map_base = mmap(NULL,
-			page_size * 2 /* in case value spans page */,
+			map_length,
 			argv[3] ? (PROT_READ | PROT_WRITE) : PROT_READ,
 			MAP_SHARED,
 			fd,
-			target & ~(off_t)(page_size - 1));
+			target & page_mask);
 	if (map_base == MAP_FAILED)
 		bb_perror_msg_and_die("mmap");
 
 //	printf("Memory mapped at address %p.\n", map_base);
 
-	virt_addr = (char*)map_base + (target & (page_size - 1));
+	virt_addr = (char*)map_base + (target & page_mask);
 
 	if (!argv[3]) {
 		switch (width) {
@@ -119,7 +123,7 @@ int devmem_main(int argc UNUSED_PARAM, char **argv)
 	}
 
 	if (ENABLE_FEATURE_CLEAN_UP) {
-		if (munmap(map_base, page_size * 2) == -1)
+		if (munmap(map_base, map_length) == -1)
 			bb_perror_msg_and_die("munmap");
 		close(fd);
 	}

Two points: you might as well compute the page span properly, if you're 
going to touch this code, and of course you do need to unmap what you've 
just mapped!

*Think* I've got the calculation right, but as I said, completely 
untested, sorry.


More information about the busybox mailing list