[bpf-next,v1] bpf: Add bpf_copy_from_user_str() helper

Commit Message

Jordan Rome Aug. 8, 2024, 5:31 p.m. UTC

This adds a helper for bpf programs to copy a user string
in a sleepable context (one that can page fault).

This matches the non-sleepable 'bpf_probe_read_user_str'.

Signed-off-by: Jordan Rome <[e-mail]>
---
 include/linux/bpf.h                           |  1 +
 include/uapi/linux/bpf.h                      | 22 +++++++++++++++++++
 kernel/bpf/helpers.c                          | 21 ++++++++++++++++++
 kernel/trace/bpf_trace.c                      |  2 ++
 tools/include/uapi/linux/bpf.h                | 22 +++++++++++++++++++
 .../selftests/bpf/prog_tests/attach_probe.c   |  2 ++
 .../selftests/bpf/prog_tests/read_vsyscall.c  |  1 +
 .../selftests/bpf/progs/read_vsyscall.c       |  3 ++-
 .../selftests/bpf/progs/test_attach_probe.c   | 17 ++++++++++++++
 9 files changed, 90 insertions(+), 1 deletion(-)

--
2.43.5

Comments

Alexei Starovoitov Aug. 8, 2024, 5:44 p.m. UTC | #1

On Thu, Aug 8, 2024 at 10:37 AM Jordan Rome <[e-mail]> wrote:
>
>  #define ___BPF_FUNC_MAPPER(FN, ctx...)                 \
>         FN(unspec, 0, ##ctx)                            \
> @@ -6006,6 +6027,7 @@ union bpf_attr {
>         FN(user_ringbuf_drain, 209, ##ctx)              \
>         FN(cgrp_storage_get, 210, ##ctx)                \
>         FN(cgrp_storage_delete, 211, ##ctx)             \
> +       FN(copy_from_user_str, 212, ##ctx)              \

Sorry, no new helpers. We can only add kfuncs now.

pw-bot: cr

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b9425e410bcb..15963f85c016 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3235,6 +3235,7 @@  extern const struct bpf_func_proto bpf_skc_to_udp6_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_unix_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto;
 extern const struct bpf_func_proto bpf_copy_from_user_proto;
+extern const struct bpf_func_proto bpf_copy_from_user_str_proto;
 extern const struct bpf_func_proto bpf_snprintf_btf_proto;
 extern const struct bpf_func_proto bpf_snprintf_proto;
 extern const struct bpf_func_proto bpf_per_cpu_ptr_proto;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 35bcf52dbc65..ee94e6b55224 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -4217,6 +4217,8 @@  union bpf_attr {
  * 		*current*\ **->mm->arg_start** and *current*\
  * 		**->mm->env_start**: using this helper and the return value,
  * 		one can quickly iterate at the right offset of the memory area.
+ *
+ *		For sleepable programs use **bpf_copy_from_user_str**\ ().
  * 	Return
  * 		On success, the strictly positive length of the output string,
  * 		including the trailing NUL character. On error, a negative
@@ -5792,6 +5794,25 @@  union bpf_attr {
  *		0 on success.
  *
  *		**-ENOENT** if the bpf_local_storage cannot be found.
+ *
+ * long bpf_copy_from_user_str(void *dst, u32 size, const void *user_ptr)
+ * 	Description
+ * 		Copy a NUL terminated string from an unsafe user address
+ * 		*unsafe_ptr* to *dst*. The *size* should include the
+ * 		terminating NUL byte. In case the string length is smaller than
+ * 		*size*, the target is not padded with further NUL bytes. If the
+ * 		string length is larger than *size*, just *size*-1 bytes are
+ * 		copied and the last byte is set to NUL.
+ *
+ * 		On success, returns the number of bytes that were written,
+ * 		including the terminal NUL. See **bpf_probe_read_user_str**\ () for
+ * 		examples of why this is better than **bpf_copy_from_user**\ ().
+ *
+ *		This helper can only be used by sleepable programs.
+ * 	Return
+ * 		On success, the strictly positive length of the output string,
+ * 		including the trailing NUL character. On error, a negative
+ * 		value.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6006,6 +6027,7 @@  union bpf_attr {
 	FN(user_ringbuf_drain, 209, ##ctx)		\
 	FN(cgrp_storage_get, 210, ##ctx)		\
 	FN(cgrp_storage_delete, 211, ##ctx)		\
+	FN(copy_from_user_str, 212, ##ctx)		\
 	/* */

 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index d02ae323996b..418c6a545d64 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -676,6 +676,27 @@  const struct bpf_func_proto bpf_copy_from_user_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };

+BPF_CALL_3(bpf_copy_from_user_str, void *, dst, u32, size,
+	   const void __user *, user_ptr)
+{
+	int ret = strncpy_from_user(dst, user_ptr, size);
+
+	if (unlikely(ret < 0))
+		memset(dst, 0, size);
+
+	return ret;
+}
+
+const struct bpf_func_proto bpf_copy_from_user_str_proto = {
+	.func		= bpf_copy_from_user_str,
+	.gpl_only	= false,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
+	.arg3_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size,
 	   const void __user *, user_ptr, struct task_struct *, tsk, u64, flags)
 {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d557bb11e0ff..d890879b10b7 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1533,6 +1533,8 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_task_stack_proto;
 	case BPF_FUNC_copy_from_user:
 		return &bpf_copy_from_user_proto;
+	case BPF_FUNC_copy_from_user_str:
+		return &bpf_copy_from_user_str_proto;
 	case BPF_FUNC_copy_from_user_task:
 		return &bpf_copy_from_user_task_proto;
 	case BPF_FUNC_snprintf_btf:
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 35bcf52dbc65..7cde1c21ef56 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -4217,6 +4217,8 @@  union bpf_attr {
  * 		*current*\ **->mm->arg_start** and *current*\
  * 		**->mm->env_start**: using this helper and the return value,
  * 		one can quickly iterate at the right offset of the memory area.
+ *
+ *		For sleepable programs use **bpf_copy_from_user_str**\ ().
  * 	Return
  * 		On success, the strictly positive length of the output string,
  * 		including the trailing NUL character. On error, a negative
@@ -5792,6 +5794,25 @@  union bpf_attr {
  *		0 on success.
  *
  *		**-ENOENT** if the bpf_local_storage cannot be found.
+ *
+ * long bpf_copy_from_user_str(void *dst, u32 size, const void *user_ptr)
+ *	Description
+ *		Copy a NUL terminated string from an unsafe user address
+ *		*unsafe_ptr* to *dst*. The *size* should include the
+ *		terminating NUL byte. In case the string length is smaller than
+ *		*size*, the target is not padded with further NUL bytes. If the
+ *		string length is larger than *size*, just *size*-1 bytes are
+ *		copied and the last byte is set to NUL.
+ *
+ *		On success, returns the number of bytes that were written,
+ *		including the terminal NUL. See **bpf_probe_read_user_str**\ () for
+ *		examples of why this is better than **bpf_copy_from_user**\ ().
+ *
+ *		This helper can only be used by sleepable programs.
+ *	Return
+ *		On success, the strictly positive length of the output string,
+ *		including the trailing NUL character. On error, a negative
+ *		value.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6006,6 +6027,7 @@  union bpf_attr {
 	FN(user_ringbuf_drain, 209, ##ctx)		\
 	FN(cgrp_storage_get, 210, ##ctx)		\
 	FN(cgrp_storage_delete, 211, ##ctx)		\
+	FN(copy_from_user_str, 212, ##ctx)		\
 	/* */

 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index 7175af39134f..6c0c047fd527 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -286,6 +286,8 @@  static void test_uprobe_sleepable(struct test_attach_probe *skel)
 	ASSERT_EQ(skel->bss->uprobe_byname3_res, 10, "check_uprobe_byname3_res");
 	ASSERT_EQ(skel->bss->uretprobe_byname3_sleepable_res, 11, "check_uretprobe_byname3_sleepable_res");
 	ASSERT_EQ(skel->bss->uretprobe_byname3_res, 12, "check_uretprobe_byname3_res");
+	ASSERT_EQ(skel->bss->uprobe_byname3_sleepable_res_str, 13, "check_uprobe_byname3_sleepable_res_str");
+	ASSERT_EQ(skel->bss->uretprobe_byname3_sleepable_res_str, 14, "check_uretprobe_byname3_sleepable_res_str");
 }

 void test_attach_probe(void)
diff --git a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
index 3405923fe4e6..26bd927fb438 100644
--- a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
@@ -22,6 +22,7 @@  struct read_ret_desc {
 	{ .name = "probe_read_user", .ret = -EFAULT },
 	{ .name = "probe_read_user_str", .ret = -EFAULT },
 	{ .name = "copy_from_user", .ret = -EFAULT },
+	{ .name = "copy_from_user_str", .ret = -EFAULT },
 	{ .name = "copy_from_user_task", .ret = -EFAULT },
 };

diff --git a/tools/testing/selftests/bpf/progs/read_vsyscall.c b/tools/testing/selftests/bpf/progs/read_vsyscall.c
index 986f96687ae1..c601592c4660 100644
--- a/tools/testing/selftests/bpf/progs/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/progs/read_vsyscall.c
@@ -7,7 +7,7 @@ 

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

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

@@ -40,6 +40,7 @@  int do_copy_from_user(void *ctx)
 	read_ret[6] = bpf_copy_from_user(buf, sizeof(buf), user_ptr);
 	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(buf, sizeof(buf), user_ptr);

 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index 68466a6ad18c..c6cefb2f916c 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -14,8 +14,10 @@  int uretprobe_byname_res = 0;
 int uprobe_byname2_res = 0;
 int uretprobe_byname2_res = 0;
 int uprobe_byname3_sleepable_res = 0;
+int uprobe_byname3_sleepable_res_str = 0;
 int uprobe_byname3_res = 0;
 int uretprobe_byname3_sleepable_res = 0;
+int uretprobe_byname3_sleepable_res_str = 0;
 int uretprobe_byname3_res = 0;
 void *user_ptr = 0;

@@ -87,11 +89,24 @@  static __always_inline bool verify_sleepable_user_copy(void)
 	return bpf_strncmp(data, sizeof(data), "test_data") == 0;
 }

+static __always_inline bool verify_sleepable_user_str_copy(void)
+{
+	int ret;
+	char data[9];
+
+	ret = bpf_copy_from_user_str(data, sizeof(data), user_ptr);
+
+	return bpf_strncmp(data, sizeof(data), "test_data") == 0 && ret == 9;
+}
+
+
 SEC("uprobe.s//proc/self/exe:trigger_func3")
 int handle_uprobe_byname3_sleepable(struct pt_regs *ctx)
 {
 	if (verify_sleepable_user_copy())
 		uprobe_byname3_sleepable_res = 9;
+	if (verify_sleepable_user_str_copy())
+		uprobe_byname3_sleepable_res_str = 13;
 	return 0;
 }

@@ -111,6 +126,8 @@  int handle_uretprobe_byname3_sleepable(struct pt_regs *ctx)
 {
 	if (verify_sleepable_user_copy())
 		uretprobe_byname3_sleepable_res = 11;
+	if (verify_sleepable_user_str_copy())
+		uretprobe_byname3_sleepable_res_str = 14;
 	return 0;
 }