mm: fix process_vm_rw page counts
authorYork Jasper Niebuhr <yjnworkstation@gmail.com>
Sat, 11 Nov 2023 18:48:59 +0000 (19:48 +0100)
committerAndrew Morton <akpm@linux-foundation.org>
Mon, 11 Dec 2023 00:51:39 +0000 (16:51 -0800)
1. There is a "-1" missing in the page number calculation in
   process_vm_rw_core.  While this can't break anything, it can cause
   unnecessary allocations in certain cases:

   Consider handling an iovec ranging over PVM_MAX_PP_ARRAY_COUNT pages
   that is also aligned to a page boundary.  While pp_stack could hold
   references to such an amount of pinned pages, nr_pages yields
   (PVM_MAX_PP_ARRAY + 1) in process_vm_rw_core.  Consequently, a larger
   buffer is allocated with kmalloc for no reason.

   For any page boundary aligned iovec that is a multiple of PAGE_SIZE
   and larger than PVM_MAX_PP_ARRAY_COUNT pages, nr_pages will be too big
   by 1 and thus kmalloc allocates excess space for one more pointer.

2. max_pages_per_loop is constant and there is no reason to have it as
   a variable.  A macro does the job just fine and saves memory.

3. Replaced "sizeof(struct pages *)" with "sizeof(struct page *)" to
   have matching types for allocation and prevent confusion.

Link: https://lkml.kernel.org/r/20231111184859.44264-1-yjnworkstation@gmail.com
Signed-off-by: York Jasper Niebuhr <yjnworkstation@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
mm/process_vm_access.c

index 0523edab03a6a575f2142df38e9391b535875a9d..b308e96cd05a28f1bac259169fcf98a969294cf9 100644 (file)
@@ -53,7 +53,10 @@ static int process_vm_rw_pages(struct page **pages,
 }
 
 /* Maximum number of pages kmalloc'd to hold struct page's during copy */
-#define PVM_MAX_KMALLOC_PAGES (PAGE_SIZE * 2)
+#define PVM_MAX_KMALLOC_PAGES 2
+
+/* Maximum number of pages that can be stored at a time */
+#define PVM_MAX_USER_PAGES (PVM_MAX_KMALLOC_PAGES * PAGE_SIZE / sizeof(struct page *))
 
 /**
  * process_vm_rw_single_vec - read/write pages from task specified
@@ -79,8 +82,6 @@ static int process_vm_rw_single_vec(unsigned long addr,
        unsigned long start_offset = addr - pa;
        unsigned long nr_pages;
        ssize_t rc = 0;
-       unsigned long max_pages_per_loop = PVM_MAX_KMALLOC_PAGES
-               / sizeof(struct pages *);
        unsigned int flags = 0;
 
        /* Work out address and page range required */
@@ -92,7 +93,7 @@ static int process_vm_rw_single_vec(unsigned long addr,
                flags |= FOLL_WRITE;
 
        while (!rc && nr_pages && iov_iter_count(iter)) {
-               int pinned_pages = min(nr_pages, max_pages_per_loop);
+               int pinned_pages = min_t(unsigned long, nr_pages, PVM_MAX_USER_PAGES);
                int locked = 1;
                size_t bytes;
 
@@ -171,7 +172,7 @@ static ssize_t process_vm_rw_core(pid_t pid, struct iov_iter *iter,
                iov_len = rvec[i].iov_len;
                if (iov_len > 0) {
                        nr_pages_iov = ((unsigned long)rvec[i].iov_base
-                                       + iov_len)
+                                       + iov_len - 1)
                                / PAGE_SIZE - (unsigned long)rvec[i].iov_base
                                / PAGE_SIZE + 1;
                        nr_pages = max(nr_pages, nr_pages_iov);
@@ -184,8 +185,8 @@ static ssize_t process_vm_rw_core(pid_t pid, struct iov_iter *iter,
        if (nr_pages > PVM_MAX_PP_ARRAY_COUNT) {
                /* For reliability don't try to kmalloc more than
                   2 pages worth */
-               process_pages = kmalloc(min_t(size_t, PVM_MAX_KMALLOC_PAGES,
-                                             sizeof(struct pages *)*nr_pages),
+               process_pages = kmalloc(min_t(size_t, PVM_MAX_KMALLOC_PAGES * PAGE_SIZE,
+                                             sizeof(struct page *)*nr_pages),
                                        GFP_KERNEL);
 
                if (!process_pages)