[bpf-next] bpf: add BPF_F_TOKEN_FD flag to pass with BPF token FD

Commit Message

Andrii Nakryiko Dec. 19, 2023, 5:31 a.m. UTC

Add BPF_F_TOKEN_FD flag to be used across bpf() syscall commands
that accept BPF token FD: BPF_PROG_LOAD, BPF_MAP_CREATE, and
BPF_BTF_LOAD. This flag has to be set whenever token FD is provided.

BPF_BTF_LOAD command didn't have a flags field, so add it as well.

Signed-off-by: Andrii Nakryiko <[e-mail]>
---
 include/uapi/linux/bpf.h       | 10 +++++++---
 kernel/bpf/syscall.c           | 19 +++++++++++++++----
 tools/include/uapi/linux/bpf.h | 10 +++++++---
 3 files changed, 29 insertions(+), 10 deletions(-)

Comments

Christian Brauner Dec. 20, 2023, 12:10 p.m. UTC | #1

On Mon, Dec 18, 2023 at 09:31:50PM -0800, Andrii Nakryiko wrote:
> Add BPF_F_TOKEN_FD flag to be used across bpf() syscall commands
> that accept BPF token FD: BPF_PROG_LOAD, BPF_MAP_CREATE, and
> BPF_BTF_LOAD. This flag has to be set whenever token FD is provided.
> 
> BPF_BTF_LOAD command didn't have a flags field, so add it as well.
> 
> Signed-off-by: Andrii Nakryiko <[e-mail]>
> ---

Reviewed-by: Christian Brauner <[e-mail]>

>  /* Get path from provided FD in BPF_OBJ_PIN/BPF_OBJ_GET commands */
>  	BPF_F_PATH_FD		= (1U << 14),
> +
> +/* BPF token FD is passed in a corresponding command's token_fd field */
> +	BPF_F_TOKEN_FD		= (1U << 15),

The placement of the new flag right after the BPF_F_PATH_FD flag alone
does tell us everything about the "we didn't know" claims wrt to the
token fd stuff. Literally the same review and the same solution I
requested back then.

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e0545201b55f..539dc19d8d11 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1364,6 +1364,9 @@  enum {
 
 /* Get path from provided FD in BPF_OBJ_PIN/BPF_OBJ_GET commands */
 	BPF_F_PATH_FD		= (1U << 14),
+
+/* BPF token FD is passed in a corresponding command's token_fd field */
+	BPF_F_TOKEN_FD		= (1U << 15),
 };
 
 /* Flags for BPF_PROG_QUERY. */
@@ -1437,7 +1440,7 @@  union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
-		__u32	map_token_fd;
+		__s32	map_token_fd;
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
@@ -1507,7 +1510,7 @@  union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		log_true_size;
-		__u32		prog_token_fd;
+		__s32		prog_token_fd;
 	};
 
 	struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -1620,7 +1623,8 @@  union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		btf_log_true_size;
-		__u32		btf_token_fd;
+		__u32		btf_flags;
+		__s32		btf_token_fd;
 	};
 
 	struct {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8faa1a20edf8..d6337842006d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1137,6 +1137,7 @@  static int map_create(union bpf_attr *attr)
 	int numa_node = bpf_map_attr_numa_node(attr);
 	u32 map_type = attr->map_type;
 	struct bpf_map *map;
+	bool token_flag;
 	int f_flags;
 	int err;
 
@@ -1144,6 +1145,12 @@  static int map_create(union bpf_attr *attr)
 	if (err)
 		return -EINVAL;
 
+	/* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
+	 * to avoid per-map type checks tripping on unknown flag
+	 */
+	token_flag = attr->map_flags & BPF_F_TOKEN_FD;
+	attr->map_flags &= ~BPF_F_TOKEN_FD;
+
 	if (attr->btf_vmlinux_value_type_id) {
 		if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS ||
 		    attr->btf_key_type_id || attr->btf_value_type_id)
@@ -1184,7 +1191,7 @@  static int map_create(union bpf_attr *attr)
 	if (!ops->map_mem_usage)
 		return -EINVAL;
 
-	if (attr->map_token_fd) {
+	if (token_flag) {
 		token = bpf_token_get_from_fd(attr->map_token_fd);
 		if (IS_ERR(token))
 			return PTR_ERR(token);
@@ -2641,12 +2648,13 @@  static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 				 BPF_F_TEST_RND_HI32 |
 				 BPF_F_XDP_HAS_FRAGS |
 				 BPF_F_XDP_DEV_BOUND_ONLY |
-				 BPF_F_TEST_REG_INVARIANTS))
+				 BPF_F_TEST_REG_INVARIANTS |
+				 BPF_F_TOKEN_FD))
 		return -EINVAL;
 
 	bpf_prog_load_fixup_attach_type(attr);
 
-	if (attr->prog_token_fd) {
+	if (attr->prog_flags & BPF_F_TOKEN_FD) {
 		token = bpf_token_get_from_fd(attr->prog_token_fd);
 		if (IS_ERR(token))
 			return PTR_ERR(token);
@@ -4837,7 +4845,10 @@  static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_
 	if (CHECK_ATTR(BPF_BTF_LOAD))
 		return -EINVAL;
 
-	if (attr->btf_token_fd) {
+	if (attr->btf_flags & ~BPF_F_TOKEN_FD)
+		return -EINVAL;
+
+	if (attr->btf_flags & BPF_F_TOKEN_FD) {
 		token = bpf_token_get_from_fd(attr->btf_token_fd);
 		if (IS_ERR(token))
 			return PTR_ERR(token);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e0545201b55f..539dc19d8d11 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1364,6 +1364,9 @@  enum {
 
 /* Get path from provided FD in BPF_OBJ_PIN/BPF_OBJ_GET commands */
 	BPF_F_PATH_FD		= (1U << 14),
+
+/* BPF token FD is passed in a corresponding command's token_fd field */
+	BPF_F_TOKEN_FD		= (1U << 15),
 };
 
 /* Flags for BPF_PROG_QUERY. */
@@ -1437,7 +1440,7 @@  union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
-		__u32	map_token_fd;
+		__s32	map_token_fd;
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
@@ -1507,7 +1510,7 @@  union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		log_true_size;
-		__u32		prog_token_fd;
+		__s32		prog_token_fd;
 	};
 
 	struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -1620,7 +1623,8 @@  union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		btf_log_true_size;
-		__u32		btf_token_fd;
+		__u32		btf_flags;
+		__s32		btf_token_fd;
 	};
 
 	struct {