linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [bpf-next v4 1/3] mm: add copy_remote_vm_str
@ 2025-01-26 12:41 Jordan Rome
  2025-01-26 12:41 ` [bpf-next v4 2/3] bpf: Add bpf_copy_from_user_task_str kfunc Jordan Rome
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jordan Rome @ 2025-01-26 12:41 UTC (permalink / raw)
  To: bpf
  Cc: linux-mm, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Kernel Team, Andrew Morton, Shakeel Butt, Alexander Potapenko

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(+)

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
--
2.43.5



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bpf-next v4 2/3] bpf: Add bpf_copy_from_user_task_str kfunc
  2025-01-26 12:41 [bpf-next v4 1/3] mm: add copy_remote_vm_str Jordan Rome
@ 2025-01-26 12:41 ` Jordan Rome
  2025-01-26 12:41 ` [bpf-next v4 3/3] selftests/bpf: Add tests for bpf_copy_from_user_task_str Jordan Rome
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jordan Rome @ 2025-01-26 12:41 UTC (permalink / raw)
  To: bpf
  Cc: linux-mm, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Kernel Team, Andrew Morton, Shakeel Butt, Alexander Potapenko

This new kfunc will be able to copy a string
from another process's/task's address space.
This is similar to `bpf_copy_from_user_str`
but accepts a `struct task_struct*` argument.

Signed-off-by: Jordan Rome <linux@jordanrome.com>
---
 kernel/bpf/helpers.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index f27ce162427a..a33f72a4c31f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3082,6 +3082,53 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
 	local_irq_restore(*flags__irq_flag);
 }

+/**
+ * bpf_copy_from_user_task_str() - Copy a string from an task's address space
+ * @dst:             Destination address, in kernel space.  This buffer must be
+ *                   at least @dst__sz bytes long.
+ * @dst__sz:         Maximum number of bytes to copy, includes the trailing NUL.
+ * @unsafe_ptr__ign: Source address in the task's address space.
+ * @tsk:             The task whose address space will be used
+ * @flags:           The only supported flag is BPF_F_PAD_ZEROS
+ *
+ * Copies a NUL terminated string from a task's address space to @dst__sz
+ * buffer. If user string is too long this will still ensure zero termination
+ * in the @dst__sz buffer unless buffer size is 0.
+ *
+ * If BPF_F_PAD_ZEROS flag is set, memset the tail of @dst__sz to 0 on success
+ * and memset all of @dst__sz on failure.
+ *
+ * Return: The number of copied bytes on success including the NUL terminator.
+ * A negative error code on failure.
+ */
+__bpf_kfunc int bpf_copy_from_user_task_str(void *dst,
+					    u32 dst__sz,
+					    const void __user *unsafe_ptr__ign,
+					    struct task_struct *tsk,
+					    u64 flags)
+{
+	int ret;
+
+	if (unlikely(flags & ~BPF_F_PAD_ZEROS))
+		return -EINVAL;
+
+	if (unlikely(!dst__sz))
+		return 0;
+
+	ret = copy_remote_vm_str(tsk, (unsigned long)unsafe_ptr__ign, dst, dst__sz, 0);
+
+	if (ret < 0) {
+		if (flags & BPF_F_PAD_ZEROS)
+			memset(dst, 0, dst__sz);
+		return ret;
+	}
+
+	if (flags & BPF_F_PAD_ZEROS)
+		memset(dst + ret, 0, dst__sz - ret);
+
+	return ret + 1;
+}
+
 __bpf_kfunc_end_defs();

 BTF_KFUNCS_START(generic_btf_ids)
@@ -3174,6 +3221,7 @@ BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
 BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_copy_from_user_str, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_copy_from_user_task_str, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_get_kmem_cache)
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
--
2.43.5



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bpf-next v4 3/3] selftests/bpf: Add tests for bpf_copy_from_user_task_str
  2025-01-26 12:41 [bpf-next v4 1/3] mm: add copy_remote_vm_str Jordan Rome
  2025-01-26 12:41 ` [bpf-next v4 2/3] bpf: Add bpf_copy_from_user_task_str kfunc Jordan Rome
@ 2025-01-26 12:41 ` Jordan Rome
  2025-01-26 14:08 ` [bpf-next v4 1/3] mm: add copy_remote_vm_str kernel test robot
  2025-01-26 14:28 ` kernel test robot
  3 siblings, 0 replies; 5+ messages in thread
From: Jordan Rome @ 2025-01-26 12:41 UTC (permalink / raw)
  To: bpf
  Cc: linux-mm, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Kernel Team, Andrew Morton, Shakeel Butt, Alexander Potapenko

This adds tests for both the happy path and the
error path (with and without the BPF_F_PAD_ZEROS flag).

Signed-off-by: Jordan Rome <linux@jordanrome.com>
---
 .../selftests/bpf/prog_tests/bpf_iter.c       |  68 +++++++++++
 .../selftests/bpf/prog_tests/read_vsyscall.c  |   1 +
 .../selftests/bpf/progs/bpf_iter_tasks.c      | 110 ++++++++++++++++++
 .../selftests/bpf/progs/read_vsyscall.c       |  11 +-
 4 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 6f1bfacd7375..add4a18c33bd 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -323,19 +323,87 @@ static void test_task_pidfd(void)
 static void test_task_sleepable(void)
 {
 	struct bpf_iter_tasks *skel;
+	int pid, status, err, data_pipe[2], finish_pipe[2], c;
+	char *test_data = NULL;
+	char *test_data_long = NULL;
+	char *data[2];
+
+	if (!ASSERT_OK(pipe(data_pipe), "data_pipe") ||
+	    !ASSERT_OK(pipe(finish_pipe), "finish_pipe"))
+		return;

 	skel = bpf_iter_tasks__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "bpf_iter_tasks__open_and_load"))
 		return;

+	pid = fork();
+	if (!ASSERT_GE(pid, 0, "fork"))
+		return;
+
+	if (pid == 0) {
+		/* child */
+		close(data_pipe[0]);
+		close(finish_pipe[1]);
+
+		test_data = malloc(sizeof(char) * 10);
+		strncpy(test_data, "test_data", 10);
+		test_data[9] = '\0';
+
+		test_data_long = malloc(sizeof(char) * 5000);
+		for (int i = 0; i < 5000; ++i) {
+			if (i % 2 == 0)
+				test_data_long[i] = 'b';
+			else
+				test_data_long[i] = 'a';
+		}
+		test_data_long[4999] = '\0';
+
+		data[0] = test_data;
+		data[1] = test_data_long;
+
+		write(data_pipe[1], &data, sizeof(data));
+
+		/* keep child alive until after the test */
+		err = read(finish_pipe[0], &c, 1);
+		if (err != 1)
+			exit(-1);
+
+		close(data_pipe[1]);
+		close(finish_pipe[0]);
+		_exit(0);
+	}
+
+	/* parent */
+	close(data_pipe[1]);
+	close(finish_pipe[0]);
+
+	err = read(data_pipe[0], &data, sizeof(data));
+	ASSERT_EQ(err, sizeof(data), "read_check");
+
+	skel->bss->user_ptr = data[0];
+	skel->bss->user_ptr_long = data[1];
+	skel->bss->pid = pid;
+
 	do_dummy_read(skel->progs.dump_task_sleepable);

 	ASSERT_GT(skel->bss->num_expected_failure_copy_from_user_task, 0,
 		  "num_expected_failure_copy_from_user_task");
 	ASSERT_GT(skel->bss->num_success_copy_from_user_task, 0,
 		  "num_success_copy_from_user_task");
+	ASSERT_GT(skel->bss->num_expected_failure_copy_from_user_task_str, 0,
+		  "num_expected_failure_copy_from_user_task_str");
+	ASSERT_GT(skel->bss->num_success_copy_from_user_task_str, 0,
+		  "num_success_copy_from_user_task_str");

 	bpf_iter_tasks__destroy(skel);
+
+	write(finish_pipe[1], &c, 1);
+	err = waitpid(pid, &status, 0);
+	ASSERT_EQ(err, pid, "waitpid");
+	ASSERT_EQ(status, 0, "zero_child_exit");
+
+	close(data_pipe[0]);
+	close(finish_pipe[1]);
 }

 static void test_task_stack(void)
diff --git a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
index c7b9ba8b1d06..a8d1eaa67020 100644
--- a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
@@ -24,6 +24,7 @@ struct read_ret_desc {
 	{ .name = "copy_from_user", .ret = -EFAULT },
 	{ .name = "copy_from_user_task", .ret = -EFAULT },
 	{ .name = "copy_from_user_str", .ret = -EFAULT },
+	{ .name = "copy_from_user_task_str", .ret = -EFAULT },
 };

 void test_read_vsyscall(void)
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
index bc10c4e4b4fa..d28d642a64c3 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
@@ -9,6 +9,13 @@ char _license[] SEC("license") = "GPL";
 uint32_t tid = 0;
 int num_unknown_tid = 0;
 int num_known_tid = 0;
+void *user_ptr = 0;
+void *user_ptr_long = 0;
+uint32_t pid = 0;
+
+static char big_str1[5000];
+static char big_str2[5005];
+static char big_str3[4996];

 SEC("iter/task")
 int dump_task(struct bpf_iter__task *ctx)
@@ -35,7 +42,9 @@ int dump_task(struct bpf_iter__task *ctx)
 }

 int num_expected_failure_copy_from_user_task = 0;
+int num_expected_failure_copy_from_user_task_str = 0;
 int num_success_copy_from_user_task = 0;
+int num_success_copy_from_user_task_str = 0;

 SEC("iter.s/task")
 int dump_task_sleepable(struct bpf_iter__task *ctx)
@@ -44,6 +53,9 @@ int dump_task_sleepable(struct bpf_iter__task *ctx)
 	struct task_struct *task = ctx->task;
 	static const char info[] = "    === END ===";
 	struct pt_regs *regs;
+	char task_str1[10] = "aaaaaaaaaa";
+	char task_str2[10], task_str3[10];
+	char task_str4[20] = "aaaaaaaaaaaaaaaaaaaa";
 	void *ptr;
 	uint32_t user_data = 0;
 	int ret;
@@ -78,8 +90,106 @@ int dump_task_sleepable(struct bpf_iter__task *ctx)
 		BPF_SEQ_PRINTF(seq, "%s\n", info);
 		return 0;
 	}
+
 	++num_success_copy_from_user_task;

+	/* Read an invalid pointer and ensure we get an error */
+	ptr = NULL;
+	ret = bpf_copy_from_user_task_str((char *)task_str1, sizeof(task_str1), ptr, task, 0);
+	if (ret >= 0 || task_str1[9] != 'a') {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Read an invalid pointer and ensure we get error with pad zeros flag */
+	ptr = NULL;
+	ret = bpf_copy_from_user_task_str((char *)task_str1, sizeof(task_str1),
+					  ptr, task, BPF_F_PAD_ZEROS);
+	if (ret >= 0 || task_str1[9] != '\0') {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	++num_expected_failure_copy_from_user_task_str;
+
+	/* Same length as the string */
+	ret = bpf_copy_from_user_task_str((char *)task_str2, 10, user_ptr, task, 0);
+	/* only need to do the task pid check once */
+	if (bpf_strncmp(task_str2, 10, "test_data\0") != 0 || ret != 10 || task->tgid != pid) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Shorter length than the string */
+	ret = bpf_copy_from_user_task_str((char *)task_str3, 2, user_ptr, task, 0);
+	if (bpf_strncmp(task_str3, 2, "t\0") != 0 || ret != 2) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Longer length than the string */
+	ret = bpf_copy_from_user_task_str((char *)task_str4, 20, user_ptr, task, 0);
+	if (bpf_strncmp(task_str4, 10, "test_data\0") != 0 || ret != 10
+	    || task_str4[sizeof(task_str4) - 1] != 'a') {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Longer length than the string with pad zeros flag */
+	ret = bpf_copy_from_user_task_str((char *)task_str4, 20, user_ptr, task, BPF_F_PAD_ZEROS);
+	if (bpf_strncmp(task_str4, 10, "test_data\0") != 0 || ret != 10
+	    || task_str4[sizeof(task_str4) - 1] != '\0') {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Longer length than the string past a page boundary */
+	ret = bpf_copy_from_user_task_str(big_str1, 5000, user_ptr, task, 0);
+	if (bpf_strncmp(big_str1, 10, "test_data\0") != 0 || ret != 10) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* String that crosses a page boundary */
+	ret = bpf_copy_from_user_task_str(big_str1, 5000, user_ptr_long, task, BPF_F_PAD_ZEROS);
+	if (bpf_strncmp(big_str1, 4, "baba") != 0 || ret != 5000
+	    || bpf_strncmp(big_str1 + 4996, 4, "bab\0") != 0) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	for (int i = 0; i < 4999; ++i) {
+		if (i % 2 == 0) {
+			if (big_str1[i] != 'b') {
+				BPF_SEQ_PRINTF(seq, "%s\n", info);
+				return 0;
+			}
+		} else {
+			if (big_str1[i] != 'a') {
+				BPF_SEQ_PRINTF(seq, "%s\n", info);
+				return 0;
+			}
+		}
+	}
+
+	/* Longer length than the string that crosses a page boundary */
+	ret = bpf_copy_from_user_task_str(big_str2, 5005, user_ptr_long, task, BPF_F_PAD_ZEROS);
+	if (bpf_strncmp(big_str2, 4, "baba") != 0 || ret != 5000
+	    || bpf_strncmp(big_str2 + 4996, 5, "bab\0\0") != 0) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	/* Shorter length than the string that crosses a page boundary */
+	ret = bpf_copy_from_user_task_str(big_str3, 4996, user_ptr_long, task, 0);
+	if (bpf_strncmp(big_str3, 4, "baba") != 0 || ret != 4996
+	    || bpf_strncmp(big_str3 + 4992, 4, "bab\0") != 0) {
+		BPF_SEQ_PRINTF(seq, "%s\n", info);
+		return 0;
+	}
+
+	++num_success_copy_from_user_task_str;
+
 	if (ctx->meta->seq_num == 0)
 		BPF_SEQ_PRINTF(seq, "    tgid      gid     data\n");

diff --git a/tools/testing/selftests/bpf/progs/read_vsyscall.c b/tools/testing/selftests/bpf/progs/read_vsyscall.c
index 39ebef430059..395591374d4f 100644
--- a/tools/testing/selftests/bpf/progs/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/progs/read_vsyscall.c
@@ -8,14 +8,16 @@

 int target_pid = 0;
 void *user_ptr = 0;
-int read_ret[9];
+int read_ret[10];

 char _license[] SEC("license") = "GPL";

 /*
- * This is the only kfunc, the others are helpers
+ * These are the kfuncs, the others are helpers
  */
 int bpf_copy_from_user_str(void *dst, u32, const void *, u64) __weak __ksym;
+int bpf_copy_from_user_task_str(void *dst, u32, const void *,
+				struct task_struct *, u64) __weak __ksym;

 SEC("fentry/" SYS_PREFIX "sys_nanosleep")
 int do_probe_read(void *ctx)
@@ -47,6 +49,11 @@ int do_copy_from_user(void *ctx)
 	read_ret[7] = bpf_copy_from_user_task(buf, sizeof(buf), user_ptr,
 					      bpf_get_current_task_btf(), 0);
 	read_ret[8] = bpf_copy_from_user_str((char *)buf, sizeof(buf), user_ptr, 0);
+	read_ret[9] = bpf_copy_from_user_task_str((char *)buf,
+						  sizeof(buf),
+						  user_ptr,
+						  bpf_get_current_task_btf(),
+						  0);

 	return 0;
 }
--
2.43.5



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bpf-next v4 1/3] mm: add copy_remote_vm_str
  2025-01-26 12:41 [bpf-next v4 1/3] mm: add copy_remote_vm_str Jordan Rome
  2025-01-26 12:41 ` [bpf-next v4 2/3] bpf: Add bpf_copy_from_user_task_str kfunc Jordan Rome
  2025-01-26 12:41 ` [bpf-next v4 3/3] selftests/bpf: Add tests for bpf_copy_from_user_task_str Jordan Rome
@ 2025-01-26 14:08 ` kernel test robot
  2025-01-26 14:28 ` kernel test robot
  3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-01-26 14:08 UTC (permalink / raw)
  To: Jordan Rome, bpf
  Cc: llvm, oe-kbuild-all, linux-mm, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Kernel Team, Andrew Morton,
	Shakeel Butt, Alexander Potapenko

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bpf-next v4 1/3] mm: add copy_remote_vm_str
  2025-01-26 12:41 [bpf-next v4 1/3] mm: add copy_remote_vm_str Jordan Rome
                   ` (2 preceding siblings ...)
  2025-01-26 14:08 ` [bpf-next v4 1/3] mm: add copy_remote_vm_str kernel test robot
@ 2025-01-26 14:28 ` kernel test robot
  3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-01-26 14:28 UTC (permalink / raw)
  To: Jordan Rome, bpf
  Cc: oe-kbuild-all, linux-mm, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Kernel Team, Andrew Morton, Shakeel Butt,
	Alexander Potapenko

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-01-26 14:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-26 12:41 [bpf-next v4 1/3] mm: add copy_remote_vm_str Jordan Rome
2025-01-26 12:41 ` [bpf-next v4 2/3] bpf: Add bpf_copy_from_user_task_str kfunc Jordan Rome
2025-01-26 12:41 ` [bpf-next v4 3/3] selftests/bpf: Add tests for bpf_copy_from_user_task_str Jordan Rome
2025-01-26 14:08 ` [bpf-next v4 1/3] mm: add copy_remote_vm_str kernel test robot
2025-01-26 14:28 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox