On Tue, 13 Apr 2010, Michael Abbott wrote:
> 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);
Ouch. Please leave this part of the patch out, leave the original line
alone!
>
> 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.
Um._______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
