[PATCH] bug in 2.4.26 mm/memory.c:map_user_kiobuf

[Bjorn Wesen]

Hi,

get_user_pages can in some circumstances return less than a complete 
mapping, without giving an error code. This fools map_user_kiobuf which 
loops over the assumed number of pages in the mapping, calling 
flush_dcache_page on all of them, which is not very good if the maplist is 
not fully populated. It also fools drivers using map_user_kiobuf and which 
also assume that you get a complete mapping (or an error code). Actually, 
iobuf->nr_pages is reduced to reflect the incomplete mapping, but 
iobuf->length is not, so I don't really know what the desired behaviour is
in the kernel<->driver communication.

Anyway, the flush loop is incorrect, and it probably doesn't hurt to 
restrict map_user_kiobuf so it returns -ENOMEM if get_user_pages can't 
return the full amount of pages, to avoid surprises downstream. 

get_user_pages seem to behave in this way during severe memory-shortage, but 
also (and more importantly) when a page-limited tmpfs is used as a substrate 
for an mmap and map_user_kiobuf and the page/size limit is hit.

/Bjorn

--- linux-2.4.26/mm/memory.c    2003-11-28 19:26:21.000000000 +0100
+++ linux-2.4.26/mm/memory-2.c  2004-05-11 13:48:10.000000000 +0200
@@ -563,12 +563,19 @@
        err = get_user_pages(current, mm, va, pgcount,
                        (rw==READ), 0, iobuf->maplist, NULL);
        up_read(&mm->mmap_sem);
-       if (err < 0) {
+       /* get_user_pages returns the amount of mapped pages,
+        * which can be less than the amount of requested pages
+        * in some cases. To avoid surprises downstream, we
+        * unmap and return an error in those cases.  -bjornw
+        */
+       if(err > 0)
+               iobuf->nr_pages = err;
+       if (err < pgcount) {
+               /* unmap depends on nr_pages being set at this point */
                unmap_kiobuf(iobuf);
                dprintk ("map_user_kiobuf: end %d\n", err);
-               return err;
+               return err < 0 ? err : -ENOMEM;
        }
-       iobuf->nr_pages = err;
        while (pgcount--) {
                /* FIXME: flush superflous for rw==READ,
                 * probably wrong function for rw==WRITE
--- linux-2.4.26/mm/memory.c	2003-11-28 19:26:21.000000000 +0100
+++ linux-2.4.26/mm/memory-2.c	2004-05-11 13:48:10.000000000 +0200
@@ -563,12 +563,19 @@
 	err = get_user_pages(current, mm, va, pgcount,
 			(rw==READ), 0, iobuf->maplist, NULL);
 	up_read(&mm->mmap_sem);
-	if (err < 0) {
+	/* get_user_pages returns the amount of mapped pages,
+	 * which can be less than the amount of requested pages
+	 * in some cases. To avoid surprises downstream, we
+	 * unmap and return an error in those cases.  -bjornw
+	 */
+	if(err > 0)
+		iobuf->nr_pages = err;
+	if (err < pgcount) {
+		/* unmap depends on nr_pages being set at this point */
 		unmap_kiobuf(iobuf);
 		dprintk ("map_user_kiobuf: end %d\n", err);
-		return err;
+		return err < 0 ? err : -ENOMEM;
 	}
-	iobuf->nr_pages = err;
 	while (pgcount--) {
 		/* FIXME: flush superflous for rw==READ,
 		 * probably wrong function for rw==WRITE