| Message ID | 20250126124147.3154108-1-linux@jordanrome.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf-next,v4,1/3] mm: add copy_remote_vm_str | expand |
Hi Jordan, kernel test robot noticed the following build errors: [auto build test ERROR on bpf-next/master] [also build test ERROR on bpf/master linus/master v6.13 next-20250124] [cannot apply to akpm-mm/mm-everything] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jordan-Rome/bpf-Add-bpf_copy_from_user_task_str-kfunc/20250126-204439 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250126124147.3154108-1-linux%40jordanrome.com patch subject: [bpf-next v4 1/3] mm: add copy_remote_vm_str config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20250126/202501262133.9mKauA5U-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250126/202501262133.9mKauA5U-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501262133.9mKauA5U-lkp@intel.com/ All errors (new ones prefixed by >>): >> mm/nommu.c:1717:2: error: use of undeclared identifier 'vma' 1717 | vma = find_vma(mm, addr); | ^ mm/nommu.c:1718:6: error: use of undeclared identifier 'vma'; did you mean 'vmap'? 1718 | if (vma) { | ^~~ | vmap mm/nommu.c:303:15: note: 'vmap' declared here 303 | EXPORT_SYMBOL(vmap); | ^ mm/nommu.c:1720:21: error: use of undeclared identifier 'vma' 1720 | if (addr + len >= vma->vm_end) | ^ mm/nommu.c:1721:10: error: use of undeclared identifier 'vma' 1721 | len = vma->vm_end - addr; | ^ mm/nommu.c:1724:7: error: use of undeclared identifier 'vma' 1724 | if (vma->vm_flags & VM_MAYREAD) { | ^ >> mm/nommu.c:1725:23: error: incompatible integer to pointer conversion passing 'unsigned long' to parameter of type 'const char *' [-Wint-conversion] 1725 | ret = strscpy(buf, addr, len); | ^~~~ include/linux/string.h:113:55: note: expanded from macro 'strscpy' 113 | CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__) | ^~~ include/linux/string.h:82:21: note: expanded from macro '__strscpy1' 82 | sized_strscpy(dst, src, size + __must_be_cstr(dst) + __must_be_cstr(src)) | ^~~ include/linux/string.h:72:43: note: passing argument to parameter here 72 | ssize_t sized_strscpy(char *, const char *, size_t); | ^ 6 errors generated. vim +/vma +1717 mm/nommu.c 1703 1704 /* 1705 * Copy a string from another process's address space as given in mm. 1706 * If there is any error return -EFAULT. 1707 */ 1708 static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, 1709 void *buf, int len) 1710 { 1711 int ret; 1712 1713 if (mmap_read_lock_killable(mm)) 1714 return -EFAULT; 1715 1716 /* the access must start within one of the target process's mappings */ > 1717 vma = find_vma(mm, addr); 1718 if (vma) { 1719 /* don't overrun this mapping */ > 1720 if (addr + len >= vma->vm_end) 1721 len = vma->vm_end - addr; 1722 1723 /* only read mappings where it is permitted */ 1724 if (vma->vm_flags & VM_MAYREAD) { > 1725 ret = strscpy(buf, addr, len); 1726 if (ret < 0) 1727 ret = len - 1; 1728 } else { 1729 ret = -EFAULT; 1730 } 1731 } else { 1732 ret = -EFAULT; 1733 } 1734 1735 mmap_read_unlock(mm); 1736 return ret; 1737 } 1738
Hi Jordan, kernel test robot noticed the following build errors: [auto build test ERROR on bpf-next/master] [also build test ERROR on bpf/master linus/master v6.13 next-20250124] [cannot apply to akpm-mm/mm-everything] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jordan-Rome/bpf-Add-bpf_copy_from_user_task_str-kfunc/20250126-204439 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250126124147.3154108-1-linux%40jordanrome.com patch subject: [bpf-next v4 1/3] mm: add copy_remote_vm_str config: arm-randconfig-001-20250126 (https://download.01.org/0day-ci/archive/20250126/202501262241.ZEkByWKM-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250126/202501262241.ZEkByWKM-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501262241.ZEkByWKM-lkp@intel.com/ All errors (new ones prefixed by >>): mm/nommu.c: In function '__copy_remote_vm_str': >> mm/nommu.c:1717:9: error: 'vma' undeclared (first use in this function); did you mean 'vmap'? 1717 | vma = find_vma(mm, addr); | ^~~ | vmap mm/nommu.c:1717:9: note: each undeclared identifier is reported only once for each function it appears in In file included from include/linux/bitmap.h:13, from include/linux/cpumask.h:12, from include/linux/smp.h:13, from include/linux/lockdep.h:14, from include/linux/spinlock.h:63, from include/linux/mmzone.h:8, from include/linux/gfp.h:7, from include/linux/mm.h:7, from mm/nommu.c:20: >> mm/nommu.c:1725:44: error: passing argument 2 of 'sized_strscpy' makes pointer from integer without a cast [-Wint-conversion] 1725 | ret = strscpy(buf, addr, len); | ^~~~ | | | long unsigned int include/linux/string.h:82:28: note: in definition of macro '__strscpy1' 82 | sized_strscpy(dst, src, size + __must_be_cstr(dst) + __must_be_cstr(src)) | ^~~ mm/nommu.c:1725:31: note: in expansion of macro 'strscpy' 1725 | ret = strscpy(buf, addr, len); | ^~~~~~~ include/linux/string.h:72:31: note: expected 'const char *' but argument is of type 'long unsigned int' 72 | ssize_t sized_strscpy(char *, const char *, size_t); | ^~~~~~~~~~~~ vim +1717 mm/nommu.c 1703 1704 /* 1705 * Copy a string from another process's address space as given in mm. 1706 * If there is any error return -EFAULT. 1707 */ 1708 static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, 1709 void *buf, int len) 1710 { 1711 int ret; 1712 1713 if (mmap_read_lock_killable(mm)) 1714 return -EFAULT; 1715 1716 /* the access must start within one of the target process's mappings */ > 1717 vma = find_vma(mm, addr); 1718 if (vma) { 1719 /* don't overrun this mapping */ 1720 if (addr + len >= vma->vm_end) 1721 len = vma->vm_end - addr; 1722 1723 /* only read mappings where it is permitted */ 1724 if (vma->vm_flags & VM_MAYREAD) { > 1725 ret = strscpy(buf, addr, len); 1726 if (ret < 0) 1727 ret = len - 1; 1728 } else { 1729 ret = -EFAULT; 1730 } 1731 } else { 1732 ret = -EFAULT; 1733 } 1734 1735 mmap_read_unlock(mm); 1736 return ret; 1737 } 1738
diff --git a/include/linux/mm.h b/include/linux/mm.h index f02925447e59..f3a05b3eb2f2 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2485,6 +2485,9 @@ 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); +extern int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr, + void *buf, int len, unsigned int gup_flags); + 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 398c031be9ba..e1ed5095b258 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -6714,6 +6714,124 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, } EXPORT_SYMBOL_GPL(access_process_vm); +/* + * 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; + + if (mmap_read_lock_killable(mm)) + return -EFAULT; + + /* Untag the address before looking up the VMA */ + 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. + */ + 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); + unmap_and_put_page(page, maddr); + + if (retval > -1) { + /* Found the end of the string */ + buf += retval; + goto out; + } + + retval = bytes - 1; + buf += retval; + + if (bytes == len) + goto out; + + /* + * Because strscpy always NUL terminates we need to + * copy the last byte in the page if we are going to + * load more pages + */ + addr += retval; + len -= retval; + copy_from_user_page(vma, + page, + addr, + buf, + maddr + (PAGE_SIZE - 1), + 1); + len -= 1; + buf += 1; + addr += 1; + } + +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. 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; + + mm = get_task_mm(tsk); + if (!mm) + return -EFAULT; + + ret = __copy_remote_vm_str(mm, addr, buf, len, gup_flags); + + mmput(mm); + + return ret; +} +EXPORT_SYMBOL_GPL(copy_remote_vm_str); + /* * Print the name of a VMA. */ diff --git a/mm/nommu.c b/mm/nommu.c index 9cb6e99215e2..480ddf50dec1 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -1701,6 +1701,72 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in } EXPORT_SYMBOL_GPL(access_process_vm); +/* + * 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) +{ + int ret; + + if (mmap_read_lock_killable(mm)) + return -EFAULT; + + /* the access must start within one of the target process's mappings */ + vma = find_vma(mm, addr); + if (vma) { + /* don't overrun this mapping */ + if (addr + len >= vma->vm_end) + len = vma->vm_end - addr; + + /* only read mappings where it is permitted */ + if (vma->vm_flags & VM_MAYREAD) { + ret = strscpy(buf, addr, len); + if (ret < 0) + ret = len - 1; + } else { + ret = -EFAULT; + } + } else { + ret = -EFAULT; + } + + 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. 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; + + mm = get_task_mm(tsk); + if (!mm) + return -EFAULT; + + ret = __copy_remote_vm_str(mm, addr, buf, len); + + mmput(mm); + + return ret; +} +EXPORT_SYMBOL_GPL(copy_remote_vm_str); + /** * nommu_shrink_inode_mappings - Shrink the shared mappings on an inode * @inode: The inode to check
Similar to `access_process_vm` but specific to strings. Also chunks reads by page and utilizes `strscpy` for handling null termination. Signed-off-by: Jordan Rome <linux@jordanrome.com> --- include/linux/mm.h | 3 ++ mm/memory.c | 118 +++++++++++++++++++++++++++++++++++++++++++++ mm/nommu.c | 66 +++++++++++++++++++++++++ 3 files changed, 187 insertions(+) -- 2.43.5