[PATCH v4 bpf-next 0/5] Add bpf_getxattr

[PATCH v4 bpf-next 0/5] Add bpf_getxattr

[KP Singh]

v3 -> v4

- Fixed issue incorrect increment of arg counter
- Removed __weak and noinline from kfunc definiton
- Some other minor fixes.

v2 -> v3

- Fixed missing prototype error
- Fixes suggested by other Joanne and Kumar.

v1 -> v2

- Used kfuncs as suggested by Alexei
- Used Benjamin Tissoires' patch from the HID v4 series to add a
  sleepable kfunc set (I sent the patch as a part of this series as it
  seems to have been dropped from v5) and acked it. Hope this is okay.
- Added support for verifying string constants to kfuncs

Foundation for building more complex security policies using the
BPF LSM as presented in LSF/MM/BPF:

 http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-xattr.pdf

Benjamin Tissoires (1):
  btf: Add a new kfunc set which allows to mark a function to be
    sleepable

KP Singh (4):
  bpf: kfunc support for ARG_PTR_TO_CONST_STR
  bpf: Allow kfuncs to be used in LSM programs
  bpf: Add a bpf_getxattr kfunc
  bpf/selftests: Add a selftest for bpf_getxattr

 include/linux/bpf_verifier.h                  |  2 +
 include/linux/btf.h                           |  2 +
 kernel/bpf/btf.c                              | 38 +++++++-
 kernel/bpf/verifier.c                         | 89 +++++++++++--------
 kernel/trace/bpf_trace.c                      | 42 +++++++++
 .../testing/selftests/bpf/prog_tests/xattr.c  | 58 ++++++++++++
 tools/testing/selftests/bpf/progs/xattr.c     | 37 ++++++++
 7 files changed, 228 insertions(+), 40 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/xattr.c

-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v4 bpf-next 1/5] btf: Add a new kfunc set which allows to mark a function to be sleepable

[KP Singh]

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

This allows to declare a kfunc as sleepable and prevents its use in
a non sleepable program.

Acked-by: KP Singh <kpsingh@kernel.org>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/btf.h |  2 ++
 kernel/bpf/btf.c    | 12 ++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 1bfed7fa0428..6e7517573d9e 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -18,6 +18,7 @@ enum btf_kfunc_type {
 	BTF_KFUNC_TYPE_RELEASE,
 	BTF_KFUNC_TYPE_RET_NULL,
 	BTF_KFUNC_TYPE_KPTR_ACQUIRE,
+	BTF_KFUNC_TYPE_SLEEPABLE,
 	BTF_KFUNC_TYPE_MAX,
 };
 
@@ -37,6 +38,7 @@ struct btf_kfunc_id_set {
 			struct btf_id_set *release_set;
 			struct btf_id_set *ret_null_set;
 			struct btf_id_set *kptr_acquire_set;
+			struct btf_id_set *sleepable_set;
 		};
 		struct btf_id_set *sets[BTF_KFUNC_TYPE_MAX];
 	};
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f08037c31dd7..668ecf61649b 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6171,7 +6171,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 	struct bpf_verifier_log *log = &env->log;
 	u32 i, nargs, ref_id, ref_obj_id = 0;
 	bool is_kfunc = btf_is_kernel(btf);
-	bool rel = false, kptr_get = false;
+	bool rel = false, kptr_get = false, sleepable = false;
 	const char *func_name, *ref_tname;
 	const struct btf_type *t, *ref_t;
 	const struct btf_param *args;
@@ -6202,9 +6202,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 	}
 
 	if (is_kfunc) {
-		/* Only kfunc can be release func */
 		rel = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
 						BTF_KFUNC_TYPE_RELEASE, func_id);
+		sleepable = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
+						      BTF_KFUNC_TYPE_SLEEPABLE, func_id);
 		kptr_get = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
 						     BTF_KFUNC_TYPE_KPTR_ACQUIRE, func_id);
 	}
@@ -6404,6 +6405,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 			func_name);
 		return -EINVAL;
 	}
+
+	if (sleepable && !env->prog->aux->sleepable) {
+		bpf_log(log, "kernel function %s is sleepable but the program is not\n",
+			func_name);
+		return -EINVAL;
+	}
+
 	/* returns argument register number > 0 in case of reference release kfunc */
 	return rel ? ref_regno : 0;
 }
-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v4 bpf-next 2/5] bpf: kfunc support for ARG_PTR_TO_CONST_STR

[KP Singh]

kfuncs can handle pointers to memory when the next argument is
the size of the memory that can be read and verify these as
ARG_CONST_SIZE_OR_ZERO

Similarly add support for string constants (const char *) and
verify it similar to ARG_PTR_TO_CONST_STR.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/bpf_verifier.h |  2 +
 kernel/bpf/btf.c             | 25 ++++++++++
 kernel/bpf/verifier.c        | 89 +++++++++++++++++++++---------------
 3 files changed, 78 insertions(+), 38 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 81b19669efba..f6d8898270d5 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -560,6 +560,8 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
 			     u32 regno);
 int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 		   u32 regno, u32 mem_size);
+int check_const_str(struct bpf_verifier_env *env,
+		    const struct bpf_reg_state *reg, int regno);
 
 /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
 static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 668ecf61649b..b31e8d8f2d4d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6162,6 +6162,23 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
 	return true;
 }
 
+static bool btf_param_is_const_str_ptr(const struct btf *btf,
+				       const struct btf_param *param)
+{
+	const struct btf_type *t;
+
+	t = btf_type_by_id(btf, param->type);
+	if (!btf_type_is_ptr(t))
+		return false;
+
+	t = btf_type_by_id(btf, t->type);
+	if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
+		return false;
+
+	t = btf_type_skip_modifiers(btf, t->type, NULL);
+	return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
+}
+
 static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 				    const struct btf *btf, u32 func_id,
 				    struct bpf_reg_state *regs,
@@ -6344,6 +6361,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		} else if (ptr_to_mem_ok) {
 			const struct btf_type *resolve_ret;
 			u32 type_size;
+			int err;
 
 			if (is_kfunc) {
 				bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);
@@ -6354,6 +6372,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 				 * When arg_mem_size is true, the pointer can be
 				 * void *.
 				 */
+				if (btf_param_is_const_str_ptr(btf, &args[i])) {
+					err = check_const_str(env, reg, regno);
+					if (err < 0)
+						return err;
+					continue;
+				}
+
 				if (!btf_type_is_scalar(ref_t) &&
 				    !__btf_type_is_scalar_struct(log, btf, ref_t, 0) &&
 				    (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a20d7736a5b2..8c1a73e77a1d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5840,6 +5840,56 @@ static u32 stack_slot_get_id(struct bpf_verifier_env *env, struct bpf_reg_state
 	return state->stack[spi].spilled_ptr.id;
 }
 
+int check_const_str(struct bpf_verifier_env *env,
+		    const struct bpf_reg_state *reg, int regno)
+{
+	struct bpf_map *map;
+	int map_off;
+	u64 map_addr;
+	char *str_ptr;
+	int err;
+
+	if (reg->type != PTR_TO_MAP_VALUE)
+		return -EACCES;
+
+	map = reg->map_ptr;
+	if (!bpf_map_is_rdonly(map)) {
+		verbose(env, "R%d does not point to a readonly map'\n", regno);
+		return -EACCES;
+	}
+
+	if (!tnum_is_const(reg->var_off)) {
+		verbose(env, "R%d is not a constant address'\n", regno);
+		return -EACCES;
+	}
+
+	if (!map->ops->map_direct_value_addr) {
+		verbose(env,
+			"no direct value access support for this map type\n");
+		return -EACCES;
+	}
+
+	err = check_map_access(env, regno, reg->off, map->value_size - reg->off,
+			       false, ACCESS_HELPER);
+	if (err)
+		return err;
+
+	map_off = reg->off + reg->var_off.value;
+	err = map->ops->map_direct_value_addr(map, &map_addr, map_off);
+	if (err) {
+		verbose(env, "direct value access on string failed\n");
+		return err;
+	}
+
+	str_ptr = (char *)(long)(map_addr);
+	if (!strnchr(str_ptr + map_off, map->value_size - map_off, 0)) {
+		verbose(env, "string is not zero-terminated\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			  struct bpf_call_arg_meta *meta,
 			  const struct bpf_func_proto *fn)
@@ -6074,44 +6124,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			return err;
 		err = check_ptr_alignment(env, reg, 0, size, true);
 	} else if (arg_type == ARG_PTR_TO_CONST_STR) {
-		struct bpf_map *map = reg->map_ptr;
-		int map_off;
-		u64 map_addr;
-		char *str_ptr;
-
-		if (!bpf_map_is_rdonly(map)) {
-			verbose(env, "R%d does not point to a readonly map'\n", regno);
-			return -EACCES;
-		}
-
-		if (!tnum_is_const(reg->var_off)) {
-			verbose(env, "R%d is not a constant address'\n", regno);
-			return -EACCES;
-		}
-
-		if (!map->ops->map_direct_value_addr) {
-			verbose(env, "no direct value access support for this map type\n");
-			return -EACCES;
-		}
-
-		err = check_map_access(env, regno, reg->off,
-				       map->value_size - reg->off, false,
-				       ACCESS_HELPER);
-		if (err)
-			return err;
-
-		map_off = reg->off + reg->var_off.value;
-		err = map->ops->map_direct_value_addr(map, &map_addr, map_off);
-		if (err) {
-			verbose(env, "direct value access on string failed\n");
-			return err;
-		}
-
-		str_ptr = (char *)(long)(map_addr);
-		if (!strnchr(str_ptr + map_off, map->value_size - map_off, 0)) {
-			verbose(env, "string is not zero-terminated\n");
-			return -EINVAL;
-		}
+		err = check_const_str(env, reg, regno);
 	} else if (arg_type == ARG_PTR_TO_KPTR) {
 		if (process_kptr_func(env, regno, meta))
 			return -EACCES;
-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v4 bpf-next 3/5] bpf: Allow kfuncs to be used in LSM programs

[KP Singh]

In preparation for the addition of bpf_getxattr kfunc.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 kernel/bpf/btf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index b31e8d8f2d4d..9f289b346790 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7260,6 +7260,7 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_STRUCT_OPS:
 		return BTF_KFUNC_HOOK_STRUCT_OPS;
 	case BPF_PROG_TYPE_TRACING:
+	case BPF_PROG_TYPE_LSM:
 		return BTF_KFUNC_HOOK_TRACING;
 	case BPF_PROG_TYPE_SYSCALL:
 		return BTF_KFUNC_HOOK_SYSCALL;
-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v4 bpf-next 4/5] bpf: Add a bpf_getxattr kfunc

[KP Singh]

LSMs like SELinux store security state in xattrs. bpf_getxattr enables
BPF LSM to implement similar functionality. In combination with
bpf_local_storage, xattrs can be used to develop more complex security
policies.

This kfunc wraps around __vfs_getxattr which can sleep and is,
therefore, limited to sleepable programs using the newly added
sleepable_set for kfuncs.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 kernel/trace/bpf_trace.c | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4be976cf7d63..87496d57b099 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -20,6 +20,7 @@
 #include <linux/fprobe.h>
 #include <linux/bsearch.h>
 #include <linux/sort.h>
+#include <linux/xattr.h>
 
 #include <net/bpf_sk_storage.h>
 
@@ -1181,6 +1182,47 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 };
 
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+		  "kfuncs that are used in tracing/LSM BPF programs");
+
+ssize_t bpf_getxattr(struct dentry *dentry, struct inode *inode,
+		     const char *name, void *value, int value__sz)
+{
+	return __vfs_getxattr(dentry, inode, name, value, value__sz);
+}
+
+__diag_pop();
+
+BTF_SET_START(bpf_trace_kfunc_ids)
+BTF_ID(func, bpf_getxattr)
+BTF_SET_END(bpf_trace_kfunc_ids)
+
+BTF_SET_START(bpf_trace_sleepable_kfunc_ids)
+BTF_ID(func, bpf_getxattr)
+BTF_SET_END(bpf_trace_sleepable_kfunc_ids)
+
+static const struct btf_kfunc_id_set bpf_trace_kfunc_set = {
+	.owner = THIS_MODULE,
+	.check_set = &bpf_trace_kfunc_ids,
+	.sleepable_set = &bpf_trace_sleepable_kfunc_ids,
+};
+
+static int __init bpf_trace_kfunc_init(void)
+{
+	int ret;
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
+					&bpf_trace_kfunc_set);
+	if (!ret)
+		return ret;
+
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM,
+					&bpf_trace_kfunc_set);
+
+}
+late_initcall(bpf_trace_kfunc_init);
+
 static const struct bpf_func_proto *
 bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
-- 
2.37.0.rc0.104.g0611611a94-goog

[PATCH v4 bpf-next 5/5] bpf/selftests: Add a selftest for bpf_getxattr

[KP Singh]

A simple test that adds an xattr on a copied /bin/ls and reads it back
when the copied ls is executed.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/xattr.c  | 58 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/xattr.c     | 37 ++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/xattr.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xattr.c b/tools/testing/selftests/bpf/prog_tests/xattr.c
new file mode 100644
index 000000000000..442b6c1aed0e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xattr.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2022 Google LLC.
+ */
+
+#include <test_progs.h>
+#include <sys/xattr.h>
+#include "xattr.skel.h"
+
+#define XATTR_NAME "security.bpf"
+#define XATTR_VALUE "test_progs"
+
+static unsigned int duration;
+
+void test_xattr(void)
+{
+	struct xattr *skel = NULL;
+	char tmp_dir_path[] = "/tmp/xattrXXXXXX";
+	char tmp_exec_path[64];
+	char cmd[256];
+	int err;
+
+	if (CHECK(!mkdtemp(tmp_dir_path), "mkdtemp",
+		  "unable to create tmpdir: %d\n", errno))
+		goto close_prog;
+
+	snprintf(tmp_exec_path, sizeof(tmp_exec_path), "%s/copy_of_ls",
+		 tmp_dir_path);
+	snprintf(cmd, sizeof(cmd), "cp /bin/ls %s", tmp_exec_path);
+	if (CHECK_FAIL(system(cmd)))
+		goto close_prog_rmdir;
+
+	if (CHECK(setxattr(tmp_exec_path, XATTR_NAME, XATTR_VALUE,
+			   sizeof(XATTR_VALUE), 0),
+		  "setxattr", "unable to setxattr: %d", errno))
+		goto close_prog_rmdir;
+
+	skel = xattr__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_load"))
+		goto close_prog_rmdir;
+
+	err = xattr__attach(skel);
+	if (!ASSERT_OK(err, "xattr__attach failed"))
+		goto close_prog_rmdir;
+
+	snprintf(cmd, sizeof(cmd), "%s -l", tmp_exec_path);
+	if (CHECK_FAIL(system(cmd)))
+		goto close_prog_rmdir;
+
+	ASSERT_EQ(skel->bss->result, 1, "xattr result");
+
+close_prog_rmdir:
+	snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir_path);
+	system(cmd);
+close_prog:
+	xattr__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/xattr.c b/tools/testing/selftests/bpf/progs/xattr.c
new file mode 100644
index 000000000000..ccc078fb8ebd
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xattr.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2022 Google LLC.
+ */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define XATTR_NAME "security.bpf"
+#define XATTR_VALUE "test_progs"
+
+__u64 result = 0;
+
+extern ssize_t bpf_getxattr(struct dentry *dentry, struct inode *inode,
+			    const char *name, void *value, int size) __ksym;
+
+SEC("lsm.s/bprm_committed_creds")
+void BPF_PROG(bprm_cc, struct linux_binprm *bprm)
+{
+	struct task_struct *current = bpf_get_current_task_btf();
+	char dir_xattr_value[64] = {0};
+	int xattr_sz = 0;
+
+	xattr_sz = bpf_getxattr(bprm->file->f_path.dentry,
+				bprm->file->f_path.dentry->d_inode, XATTR_NAME,
+				dir_xattr_value, 64);
+
+	if (xattr_sz <= 0)
+		return;
+
+	if (!bpf_strncmp(dir_xattr_value, sizeof(XATTR_VALUE), XATTR_VALUE))
+		result = 1;
+}
-- 
2.37.0.rc0.104.g0611611a94-goog

Re: [PATCH v4 bpf-next 5/5] bpf/selftests: Add a selftest for bpf_getxattr

[Andrii Nakryiko]

On Thu, Jun 23, 2022 at 9:56 PM KP Singh <kpsingh@kernel.org> wrote:
>
> A simple test that adds an xattr on a copied /bin/ls and reads it back
> when the copied ls is executed.
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  .../testing/selftests/bpf/prog_tests/xattr.c  | 58 +++++++++++++++++++
>  tools/testing/selftests/bpf/progs/xattr.c     | 37 ++++++++++++
>  2 files changed, 95 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c
>  create mode 100644 tools/testing/selftests/bpf/progs/xattr.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xattr.c b/tools/testing/selftests/bpf/prog_tests/xattr.c
> new file mode 100644
> index 000000000000..442b6c1aed0e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xattr.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2022 Google LLC.
> + */
> +
> +#include <test_progs.h>
> +#include <sys/xattr.h>
> +#include "xattr.skel.h"
> +
> +#define XATTR_NAME "security.bpf"
> +#define XATTR_VALUE "test_progs"
> +
> +static unsigned int duration;
> +
> +void test_xattr(void)
> +{
> +       struct xattr *skel = NULL;
> +       char tmp_dir_path[] = "/tmp/xattrXXXXXX";
> +       char tmp_exec_path[64];
> +       char cmd[256];
> +       int err;
> +
> +       if (CHECK(!mkdtemp(tmp_dir_path), "mkdtemp",
> +                 "unable to create tmpdir: %d\n", errno))
> +               goto close_prog;
> +
> +       snprintf(tmp_exec_path, sizeof(tmp_exec_path), "%s/copy_of_ls",
> +                tmp_dir_path);
> +       snprintf(cmd, sizeof(cmd), "cp /bin/ls %s", tmp_exec_path);
> +       if (CHECK_FAIL(system(cmd)))
> +               goto close_prog_rmdir;
> +
> +       if (CHECK(setxattr(tmp_exec_path, XATTR_NAME, XATTR_VALUE,
> +                          sizeof(XATTR_VALUE), 0),

let's not use CHECK() and CHECK_FAIL() for new tests


> +                 "setxattr", "unable to setxattr: %d", errno))
> +               goto close_prog_rmdir;
> +
> +       skel = xattr__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel_load"))
> +               goto close_prog_rmdir;
> +
> +       err = xattr__attach(skel);
> +       if (!ASSERT_OK(err, "xattr__attach failed"))
> +               goto close_prog_rmdir;
> +
> +       snprintf(cmd, sizeof(cmd), "%s -l", tmp_exec_path);
> +       if (CHECK_FAIL(system(cmd)))
> +               goto close_prog_rmdir;
> +
> +       ASSERT_EQ(skel->bss->result, 1, "xattr result");
> +
> +close_prog_rmdir:
> +       snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir_path);
> +       system(cmd);
> +close_prog:
> +       xattr__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/xattr.c b/tools/testing/selftests/bpf/progs/xattr.c
> new file mode 100644
> index 000000000000..ccc078fb8ebd
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/xattr.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2022 Google LLC.
> + */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +#define XATTR_NAME "security.bpf"
> +#define XATTR_VALUE "test_progs"
> +
> +__u64 result = 0;
> +
> +extern ssize_t bpf_getxattr(struct dentry *dentry, struct inode *inode,
> +                           const char *name, void *value, int size) __ksym;
> +
> +SEC("lsm.s/bprm_committed_creds")
> +void BPF_PROG(bprm_cc, struct linux_binprm *bprm)
> +{
> +       struct task_struct *current = bpf_get_current_task_btf();
> +       char dir_xattr_value[64] = {0};
> +       int xattr_sz = 0;
> +
> +       xattr_sz = bpf_getxattr(bprm->file->f_path.dentry,
> +                               bprm->file->f_path.dentry->d_inode, XATTR_NAME,
> +                               dir_xattr_value, 64);
> +
> +       if (xattr_sz <= 0)
> +               return;
> +
> +       if (!bpf_strncmp(dir_xattr_value, sizeof(XATTR_VALUE), XATTR_VALUE))
> +               result = 1;
> +}
> --
> 2.37.0.rc0.104.g0611611a94-goog
>

Re: [PATCH v4 bpf-next 2/5] bpf: kfunc support for ARG_PTR_TO_CONST_STR

[Andrii Nakryiko]

On Thu, Jun 23, 2022 at 9:56 PM KP Singh <kpsingh@kernel.org> wrote:
>
> kfuncs can handle pointers to memory when the next argument is
> the size of the memory that can be read and verify these as
> ARG_CONST_SIZE_OR_ZERO
>
> Similarly add support for string constants (const char *) and
> verify it similar to ARG_PTR_TO_CONST_STR.
>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
>  include/linux/bpf_verifier.h |  2 +
>  kernel/bpf/btf.c             | 25 ++++++++++
>  kernel/bpf/verifier.c        | 89 +++++++++++++++++++++---------------
>  3 files changed, 78 insertions(+), 38 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 81b19669efba..f6d8898270d5 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -560,6 +560,8 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
>                              u32 regno);
>  int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>                    u32 regno, u32 mem_size);
> +int check_const_str(struct bpf_verifier_env *env,
> +                   const struct bpf_reg_state *reg, int regno);
>
>  /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
>  static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 668ecf61649b..b31e8d8f2d4d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6162,6 +6162,23 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
>         return true;
>  }
>
> +static bool btf_param_is_const_str_ptr(const struct btf *btf,
> +                                      const struct btf_param *param)
> +{
> +       const struct btf_type *t;
> +
> +       t = btf_type_by_id(btf, param->type);
> +       if (!btf_type_is_ptr(t))
> +               return false;
> +
> +       t = btf_type_by_id(btf, t->type);
> +       if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
> +               return false;
> +
> +       t = btf_type_skip_modifiers(btf, t->type, NULL);

nit: this looks a bit fragile, you assume CONST comes first and then
skip the rest of modifiers (including typedefs). Maybe either make it
more permissive and then check that CONST is somewhere there in the
chain (you'll have to open-code btf_type_skip_modifiers() loop), or
make it more restrictive and say that it has to be `const char *` and
nothing else (no volatile, no restrict, no typedefs)?

> +       return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
> +}
> +

[...]

Re: [PATCH v4 bpf-next 2/5] bpf: kfunc support for ARG_PTR_TO_CONST_STR

[KP Singh]

On Fri, Jun 24, 2022 at 5:03 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 9:56 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > kfuncs can handle pointers to memory when the next argument is
> > the size of the memory that can be read and verify these as
> > ARG_CONST_SIZE_OR_ZERO
> >
> > Similarly add support for string constants (const char *) and
> > verify it similar to ARG_PTR_TO_CONST_STR.
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > ---
> >  include/linux/bpf_verifier.h |  2 +
> >  kernel/bpf/btf.c             | 25 ++++++++++
> >  kernel/bpf/verifier.c        | 89 +++++++++++++++++++++---------------
> >  3 files changed, 78 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index 81b19669efba..f6d8898270d5 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -560,6 +560,8 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
> >                              u32 regno);
> >  int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> >                    u32 regno, u32 mem_size);
> > +int check_const_str(struct bpf_verifier_env *env,
> > +                   const struct bpf_reg_state *reg, int regno);
> >
> >  /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
> >  static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 668ecf61649b..b31e8d8f2d4d 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -6162,6 +6162,23 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
> >         return true;
> >  }
> >
> > +static bool btf_param_is_const_str_ptr(const struct btf *btf,
> > +                                      const struct btf_param *param)
> > +{
> > +       const struct btf_type *t;
> > +
> > +       t = btf_type_by_id(btf, param->type);
> > +       if (!btf_type_is_ptr(t))
> > +               return false;
> > +
> > +       t = btf_type_by_id(btf, t->type);
> > +       if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
> > +               return false;
> > +
> > +       t = btf_type_skip_modifiers(btf, t->type, NULL);
>
> nit: this looks a bit fragile, you assume CONST comes first and then
> skip the rest of modifiers (including typedefs). Maybe either make it
> more permissive and then check that CONST is somewhere there in the
> chain (you'll have to open-code btf_type_skip_modifiers() loop), or
> make it more restrictive and say that it has to be `const char *` and
> nothing else (no volatile, no restrict, no typedefs)?

I did not bother doing that since they are kfuncs and we have a limited set of
types, but I agree that it will confuse someone, someday. So, I updated it.
Also, while I was at it, I moved the comment for the arg_mem_size below
where it should have.

Does this seem okay to you?


diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 9f289b346790..a97e664e4d4d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6166,17 +6166,21 @@ static bool btf_param_is_const_str_ptr(const
struct btf *btf,
                                       const struct btf_param *param)
 {
        const struct btf_type *t;
+       bool is_const = false;

        t = btf_type_by_id(btf, param->type);
        if (!btf_type_is_ptr(t))
                return false;

        t = btf_type_by_id(btf, t->type);
-       if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
-               return false;
+       while (btf_type_is_modifier(t)) {
+               if (BTF_INFO_KIND(t->info) == BTF_KIND_CONST)
+                       is_const = true;
+               t = btf_type_by_id(btf, t->type);
+       }

-       t = btf_type_skip_modifiers(btf, t->type, NULL);
-       return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
+       return (is_const &&
+               !strcmp(btf_name_by_offset(btf, t->name_off), "char"));
 }

 static int btf_check_func_arg_match(struct bpf_verifier_env *env,
@@ -6366,12 +6370,7 @@ static int btf_check_func_arg_match(struct
bpf_verifier_env *env,
                        if (is_kfunc) {
                                bool arg_mem_size = i + 1 < nargs &&
is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);

-                               /* Permit pointer to mem, but only when argument
-                                * type is pointer to scalar, or struct composed
-                                * (recursively) of scalars.
-                                * When arg_mem_size is true, the pointer can be
-                                * void *.
-                                */
+
                                if (btf_param_is_const_str_ptr(btf, &args[i])) {
                                        err = check_const_str(env, reg, regno);
                                        if (err < 0)
@@ -6379,6 +6378,12 @@ static int btf_check_func_arg_match(struct
bpf_verifier_env *env,
                                        continue;
                                }

+                               /* Permit pointer to mem, but only when argument
+                                * type is pointer to scalar, or struct composed
+                                * (recursively) of scalars.
+                                * When arg_mem_size is true, the pointer can be
+                                * void *.
+                                */
                                if (!btf_type_is_scalar(ref_t) &&
                                    !__btf_type_is_scalar_struct(log,
btf, ref_t, 0) &&
                                    (arg_mem_size ?
!btf_type_is_void(ref_t) : 1)) {


>
> > +       return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
> > +}
> > +
>
> [...]

Re: [PATCH v4 bpf-next 0/5] Add bpf_getxattr

[Christoph Hellwig]

This needs to have a proper discussin on linux-fsdevel.  Please
resend the whole there.  Adding the lsm list in addition would not be
bad idea either.

Re: [PATCH v4 bpf-next 2/5] bpf: kfunc support for ARG_PTR_TO_CONST_STR

[Andrii Nakryiko]

On Fri, Jun 24, 2022 at 6:26 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Fri, Jun 24, 2022 at 5:03 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Jun 23, 2022 at 9:56 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > kfuncs can handle pointers to memory when the next argument is
> > > the size of the memory that can be read and verify these as
> > > ARG_CONST_SIZE_OR_ZERO
> > >
> > > Similarly add support for string constants (const char *) and
> > > verify it similar to ARG_PTR_TO_CONST_STR.
> > >
> > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > > ---
> > >  include/linux/bpf_verifier.h |  2 +
> > >  kernel/bpf/btf.c             | 25 ++++++++++
> > >  kernel/bpf/verifier.c        | 89 +++++++++++++++++++++---------------
> > >  3 files changed, 78 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > > index 81b19669efba..f6d8898270d5 100644
> > > --- a/include/linux/bpf_verifier.h
> > > +++ b/include/linux/bpf_verifier.h
> > > @@ -560,6 +560,8 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
> > >                              u32 regno);
> > >  int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> > >                    u32 regno, u32 mem_size);
> > > +int check_const_str(struct bpf_verifier_env *env,
> > > +                   const struct bpf_reg_state *reg, int regno);
> > >
> > >  /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
> > >  static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index 668ecf61649b..b31e8d8f2d4d 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -6162,6 +6162,23 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
> > >         return true;
> > >  }
> > >
> > > +static bool btf_param_is_const_str_ptr(const struct btf *btf,
> > > +                                      const struct btf_param *param)
> > > +{
> > > +       const struct btf_type *t;
> > > +
> > > +       t = btf_type_by_id(btf, param->type);
> > > +       if (!btf_type_is_ptr(t))
> > > +               return false;
> > > +
> > > +       t = btf_type_by_id(btf, t->type);
> > > +       if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
> > > +               return false;
> > > +
> > > +       t = btf_type_skip_modifiers(btf, t->type, NULL);
> >
> > nit: this looks a bit fragile, you assume CONST comes first and then
> > skip the rest of modifiers (including typedefs). Maybe either make it
> > more permissive and then check that CONST is somewhere there in the
> > chain (you'll have to open-code btf_type_skip_modifiers() loop), or
> > make it more restrictive and say that it has to be `const char *` and
> > nothing else (no volatile, no restrict, no typedefs)?
>
> I did not bother doing that since they are kfuncs and we have a limited set of
> types, but I agree that it will confuse someone, someday. So, I updated it.
> Also, while I was at it, I moved the comment for the arg_mem_size below
> where it should have.
>
> Does this seem okay to you?
>

yep, thanks!

>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 9f289b346790..a97e664e4d4d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6166,17 +6166,21 @@ static bool btf_param_is_const_str_ptr(const
> struct btf *btf,
>                                        const struct btf_param *param)
>  {
>         const struct btf_type *t;
> +       bool is_const = false;
>
>         t = btf_type_by_id(btf, param->type);
>         if (!btf_type_is_ptr(t))
>                 return false;
>
>         t = btf_type_by_id(btf, t->type);
> -       if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
> -               return false;
> +       while (btf_type_is_modifier(t)) {
> +               if (BTF_INFO_KIND(t->info) == BTF_KIND_CONST)
> +                       is_const = true;
> +               t = btf_type_by_id(btf, t->type);
> +       }
>
> -       t = btf_type_skip_modifiers(btf, t->type, NULL);
> -       return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
> +       return (is_const &&
> +               !strcmp(btf_name_by_offset(btf, t->name_off), "char"));
>  }
>
>  static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> @@ -6366,12 +6370,7 @@ static int btf_check_func_arg_match(struct
> bpf_verifier_env *env,
>                         if (is_kfunc) {
>                                 bool arg_mem_size = i + 1 < nargs &&
> is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);
>
> -                               /* Permit pointer to mem, but only when argument
> -                                * type is pointer to scalar, or struct composed
> -                                * (recursively) of scalars.
> -                                * When arg_mem_size is true, the pointer can be
> -                                * void *.
> -                                */
> +
>                                 if (btf_param_is_const_str_ptr(btf, &args[i])) {
>                                         err = check_const_str(env, reg, regno);
>                                         if (err < 0)
> @@ -6379,6 +6378,12 @@ static int btf_check_func_arg_match(struct
> bpf_verifier_env *env,
>                                         continue;
>                                 }
>
> +                               /* Permit pointer to mem, but only when argument
> +                                * type is pointer to scalar, or struct composed
> +                                * (recursively) of scalars.
> +                                * When arg_mem_size is true, the pointer can be
> +                                * void *.
> +                                */
>                                 if (!btf_type_is_scalar(ref_t) &&
>                                     !__btf_type_is_scalar_struct(log,
> btf, ref_t, 0) &&
>                                     (arg_mem_size ?
> !btf_type_is_void(ref_t) : 1)) {
>
>
> >
> > > +       return !strcmp(btf_name_by_offset(btf, t->name_off), "char");
> > > +}
> > > +
> >
> > [...]