mm: Add copy_remote_vm_str() for readng C strings from remote VM

Similar to `access_process_vm()` but specific to strings. Also chunks
reads by page and utilizes `strscpy()` for handling null termination.

The primary motivation for this change is to copy strings from
a non-current task/process in BPF. There is already a helper
`bpf_copy_from_user_task()`, which uses `access_process_vm()` but one to
handle strings would be very helpful.

Signed-off-by: Jordan Rome <linux@jordanrome.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
Link: https://lore.kernel.org/bpf/20250213152125.1837400-1-linux@jordanrome.com

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7b1068ddcbb70b732a4f2843e6b55fd73cc3e320..5409c7b505094588bd23b3e340a1b33cada58ca2 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2486,6 +2486,11 @@ extern int access_process_vm(struct task_struct *tsk, unsigned long addr,
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
                void *buf, int len, unsigned int gup_flags);
 
+#ifdef CONFIG_BPF_SYSCALL
+extern int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr,
+                             void *buf, int len, unsigned int gup_flags);
+#endif
+
 long get_user_pages_remote(struct mm_struct *mm,
                           unsigned long start, unsigned long nr_pages,
                           unsigned int gup_flags, struct page **pages,

diff --git a/mm/memory.c b/mm/memory.c
index 539c0f7c6d5458791e723ac58e25e5b6b9f73c89..37dec50d73bc73b8f628a0ed22a3df96eab7159f 100644 (file)
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6803,6 +6803,124 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr,
 }
 EXPORT_SYMBOL_GPL(access_process_vm);
 
+#ifdef CONFIG_BPF_SYSCALL
+/*
+ * Copy a string from another process's address space as given in mm.
+ * If there is any error return -EFAULT.
+ */
+static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
+                               void *buf, int len, unsigned int gup_flags)
+{
+       void *old_buf = buf;
+       int err = 0;
+
+       *(char *)buf = '\0';
+
+       if (mmap_read_lock_killable(mm))
+               return -EFAULT;
+
+       addr = untagged_addr_remote(mm, addr);
+
+       /* Avoid triggering the temporary warning in __get_user_pages */
+       if (!vma_lookup(mm, addr)) {
+               err = -EFAULT;
+               goto out;
+       }
+
+       while (len) {
+               int bytes, offset, retval;
+               void *maddr;
+               struct page *page;
+               struct vm_area_struct *vma = NULL;
+
+               page = get_user_page_vma_remote(mm, addr, gup_flags, &vma);
+               if (IS_ERR(page)) {
+                       /*
+                        * Treat as a total failure for now until we decide how
+                        * to handle the CONFIG_HAVE_IOREMAP_PROT case and
+                        * stack expansion.
+                        */
+                       *(char *)buf = '\0';
+                       err = -EFAULT;
+                       goto out;
+               }
+
+               bytes = len;
+               offset = addr & (PAGE_SIZE - 1);
+               if (bytes > PAGE_SIZE - offset)
+                       bytes = PAGE_SIZE - offset;
+
+               maddr = kmap_local_page(page);
+               retval = strscpy(buf, maddr + offset, bytes);
+               if (retval >= 0) {
+                       /* Found the end of the string */
+                       buf += retval;
+                       unmap_and_put_page(page, maddr);
+                       break;
+               }
+
+               buf += bytes - 1;
+               /*
+                * Because strscpy always NUL terminates we need to
+                * copy the last byte in the page if we are going to
+                * load more pages
+                */
+               if (bytes != len) {
+                       addr += bytes - 1;
+                       copy_from_user_page(vma, page, addr, buf, maddr + (PAGE_SIZE - 1), 1);
+                       buf += 1;
+                       addr += 1;
+               }
+               len -= bytes;
+
+               unmap_and_put_page(page, maddr);
+       }
+
+out:
+       mmap_read_unlock(mm);
+       if (err)
+               return err;
+       return buf - old_buf;
+}
+
+/**
+ * copy_remote_vm_str - copy a string from another process's address space.
+ * @tsk:       the task of the target address space
+ * @addr:      start address to read from
+ * @buf:       destination buffer
+ * @len:       number of bytes to copy
+ * @gup_flags: flags modifying lookup behaviour
+ *
+ * The caller must hold a reference on @mm.
+ *
+ * Return: number of bytes copied from @addr (source) to @buf (destination);
+ * not including the trailing NUL. Always guaranteed to leave NUL-terminated
+ * buffer. On any error, return -EFAULT.
+ */
+int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr,
+                      void *buf, int len, unsigned int gup_flags)
+{
+       struct mm_struct *mm;
+       int ret;
+
+       if (unlikely(len == 0))
+               return 0;
+
+       mm = get_task_mm(tsk);
+       if (!mm) {
+               *(char *)buf = '\0';
+               return -EFAULT;
+       }
+
+       ret = __copy_remote_vm_str(mm, addr, buf, len, gup_flags);
+
+       mmput(mm);
+
+       return ret;
+}
+EXPORT_SYMBOL_GPL(copy_remote_vm_str);
+#endif /* CONFIG_BPF_SYSCALL */
+
 /*
  * Print the name of a VMA.
  */

diff --git a/mm/nommu.c b/mm/nommu.c
index baa79abdaf037c92c0447169702424cf13357859..0e882a88367a1b9dd0138a29f6ad1bd172df22f6 100644 (file)
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1708,6 +1708,85 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
 }
 EXPORT_SYMBOL_GPL(access_process_vm);
 
+#ifdef CONFIG_BPF_SYSCALL
+/*
+ * Copy a string from another process's address space as given in mm.
+ * If there is any error return -EFAULT.
+ */
+static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
+                               void *buf, int len)
+{
+       unsigned long addr_end;
+       struct vm_area_struct *vma;
+       int ret = -EFAULT;
+
+       *(char *)buf = '\0';
+
+       if (mmap_read_lock_killable(mm))
+               return ret;
+
+       /* the access must start within one of the target process's mappings */
+       vma = find_vma(mm, addr);
+       if (!vma)
+               goto out;
+
+       if (check_add_overflow(addr, len, &addr_end))
+               goto out;
+
+       /* don't overrun this mapping */
+       if (addr_end > vma->vm_end)
+               len = vma->vm_end - addr;
+
+       /* only read mappings where it is permitted */
+       if (vma->vm_flags & VM_MAYREAD) {
+               ret = strscpy(buf, (char *)addr, len);
+               if (ret < 0)
+                       ret = len - 1;
+       }
+
+out:
+       mmap_read_unlock(mm);
+       return ret;
+}
+
+/**
+ * copy_remote_vm_str - copy a string from another process's address space.
+ * @tsk:       the task of the target address space
+ * @addr:      start address to read from
+ * @buf:       destination buffer
+ * @len:       number of bytes to copy
+ * @gup_flags: flags modifying lookup behaviour (unused)
+ *
+ * The caller must hold a reference on @mm.
+ *
+ * Return: number of bytes copied from @addr (source) to @buf (destination);
+ * not including the trailing NUL. Always guaranteed to leave NUL-terminated
+ * buffer. On any error, return -EFAULT.
+ */
+int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr,
+                      void *buf, int len, unsigned int gup_flags)
+{
+       struct mm_struct *mm;
+       int ret;
+
+       if (unlikely(len == 0))
+               return 0;
+
+       mm = get_task_mm(tsk);
+       if (!mm) {
+               *(char *)buf = '\0';
+               return -EFAULT;
+       }
+
+       ret = __copy_remote_vm_str(mm, addr, buf, len);
+
+       mmput(mm);
+
+       return ret;
+}
+EXPORT_SYMBOL_GPL(copy_remote_vm_str);
+#endif /* CONFIG_BPF_SYSCALL */
+
 /**
  * nommu_shrink_inode_mappings - Shrink the shared mappings on an inode
  * @inode: The inode to check