Commit 6e5cae9d authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf-additional-use-cases-for-untrusted-ptr_to_mem'

Eduard Zingerman says:

====================
bpf: additional use-cases for untrusted PTR_TO_MEM

This patch set introduces two usability enhancements leveraging
untrusted pointers to mem:
- When reading a pointer field from a PTR_TO_BTF_ID, the resulting
  value is now assumed to be PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED
  instead of SCALAR_VALUE, provided the pointer points to a primitive
  type.
- __arg_untrusted attribute for global function parameters,
  allowed for pointer arguments of both structural and primitive
  types:
  - For structural types, the attribute produces
    PTR_TO_BTF_ID|PTR_UNTRUSTED.
  - For primitive types, it yields
    PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED.

Here are examples enabled by the series:

  struct foo {
    int *arr;
  };
  ...
  p = bpf_core_cast(..., struct foo);
  bpf_for(i, 0, ...) {
    ... p->arr[i] ...       // load at any offset is allowed
  }

  int memcmp(void *a __arg_untrusted, void *b __arg_untrusted, size_t n) {
    bpf_for(i, 0, n)
      if (a[i] - b[i])      // load at any offset is allowed
        return ...;
    return 0;
  }

The patch-set was inspired by Anrii's series [1]. The goal of that
series was to define a generic global glob_match function, capable to
accept any pointer type:

  __weak int glob_match(const char *pat, const char *str);

  char filename_glob[32];

  void foo(...) {
    ...
    task = bpf_get_current_task_btf();
    filename = task->mm->exe_file->f_path.dentry->d_name.name;
    ... match_glob(filename_glob,  // pointer to map value
                   filename) ...   // scalar
  }

At the moment, there is no straightforward way to express such a
function. This patch-set makes it possible to define it as follows:

  __weak int glob_match(const char *pat __arg_untrusted,
                        const char *str __arg_untrusted);

[1] https://github.com/anakryiko/linux/tree/bpf-mem-cast

Changelog:
v1: https://lore.kernel.org/bpf/20250702224209.3300396-1-eddyz87@gmail.com/
v1 -> v2:
- Added safety check in btf_prepare_func_args() to ensure that only
  struct or primitive types could be combined with __arg_untrusted
  (Alexei).
- Removed unnecessary 'continue' btf_check_func_arg_match() (Alexei).
- Added test cases for __arg_untrusted pointers to enum and
  __arg_untrusted combined with non-kernel type (Kumar).
- Added acks from Kumar.
====================

Link: https://patch.msgid.link/20250704230354.1323244-1-eddyz87@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 03fe01dd 68cca81f
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -223,6 +223,7 @@ u32 btf_nr_types(const struct btf *btf);
struct btf *btf_base_btf(const struct btf *btf);
bool btf_type_is_i32(const struct btf_type *t);
bool btf_type_is_i64(const struct btf_type *t);
bool btf_type_is_primitive(const struct btf_type *t);
bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
			   const struct btf_member *m,
			   u32 expected_offset, u32 expected_size);
+52 −5
Original line number Diff line number Diff line
@@ -891,6 +891,12 @@ bool btf_type_is_i64(const struct btf_type *t)
	return btf_type_is_int(t) && __btf_type_int_is_regular(t, 8);
}

bool btf_type_is_primitive(const struct btf_type *t)
{
	return (btf_type_is_int(t) && btf_type_int_is_regular(t)) ||
	       btf_is_any_enum(t);
}

/*
 * Check that given struct member is a regular int with expected
 * offset and size.
@@ -6915,6 +6921,7 @@ enum bpf_struct_walk_result {
	/* < 0 error */
	WALK_SCALAR = 0,
	WALK_PTR,
	WALK_PTR_UNTRUSTED,
	WALK_STRUCT,
};

@@ -7156,6 +7163,8 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
					*field_name = mname;
				return WALK_PTR;
			}

			return WALK_PTR_UNTRUSTED;
		}

		/* Allow more flexible access within an int as long as
@@ -7228,6 +7237,9 @@ int btf_struct_access(struct bpf_verifier_log *log,
			*next_btf_id = id;
			*flag = tmp_flag;
			return PTR_TO_BTF_ID;
		case WALK_PTR_UNTRUSTED:
			*flag = MEM_RDONLY | PTR_UNTRUSTED;
			return PTR_TO_MEM;
		case WALK_SCALAR:
			return SCALAR_VALUE;
		case WALK_STRUCT:
@@ -7643,8 +7655,9 @@ enum btf_arg_tag {
	ARG_TAG_CTX	  = BIT_ULL(0),
	ARG_TAG_NONNULL   = BIT_ULL(1),
	ARG_TAG_TRUSTED   = BIT_ULL(2),
	ARG_TAG_NULLABLE = BIT_ULL(3),
	ARG_TAG_ARENA	 = BIT_ULL(4),
	ARG_TAG_UNTRUSTED = BIT_ULL(3),
	ARG_TAG_NULLABLE  = BIT_ULL(4),
	ARG_TAG_ARENA	  = BIT_ULL(5),
};

/* Process BTF of a function to produce high-level expectation of function
@@ -7752,6 +7765,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
				tags |= ARG_TAG_CTX;
			} else if (strcmp(tag, "trusted") == 0) {
				tags |= ARG_TAG_TRUSTED;
			} else if (strcmp(tag, "untrusted") == 0) {
				tags |= ARG_TAG_UNTRUSTED;
			} else if (strcmp(tag, "nonnull") == 0) {
				tags |= ARG_TAG_NONNULL;
			} else if (strcmp(tag, "nullable") == 0) {
@@ -7812,6 +7827,38 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
			sub->args[i].btf_id = kern_type_id;
			continue;
		}
		if (tags & ARG_TAG_UNTRUSTED) {
			struct btf *vmlinux_btf;
			int kern_type_id;

			if (tags & ~ARG_TAG_UNTRUSTED) {
				bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
				return -EINVAL;
			}

			ref_t = btf_type_skip_modifiers(btf, t->type, NULL);
			if (btf_type_is_void(ref_t) || btf_type_is_primitive(ref_t)) {
				sub->args[i].arg_type = ARG_PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED;
				sub->args[i].mem_size = 0;
				continue;
			}

			kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
			if (kern_type_id < 0)
				return kern_type_id;

			vmlinux_btf = bpf_get_btf_vmlinux();
			ref_t = btf_type_by_id(vmlinux_btf, kern_type_id);
			if (!btf_type_is_struct(ref_t)) {
				tname = __btf_name_by_offset(vmlinux_btf, t->name_off);
				bpf_log(log, "arg#%d has type %s '%s', but only struct or primitive types are allowed\n",
					i, btf_type_str(ref_t), tname);
				return -EINVAL;
			}
			sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
			sub->args[i].btf_id = kern_type_id;
			continue;
		}
		if (tags & ARG_TAG_ARENA) {
			if (tags & ~ARG_TAG_ARENA) {
				bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
+54 −23
Original line number Diff line number Diff line
@@ -2796,22 +2796,33 @@ static void mark_reg_not_init(struct bpf_verifier_env *env,
	__mark_reg_not_init(env, regs + regno);
}
static void mark_btf_ld_reg(struct bpf_verifier_env *env,
static int mark_btf_ld_reg(struct bpf_verifier_env *env,
			   struct bpf_reg_state *regs, u32 regno,
			   enum bpf_reg_type reg_type,
			   struct btf *btf, u32 btf_id,
			   enum bpf_type_flag flag)
{
	if (reg_type == SCALAR_VALUE) {
	switch (reg_type) {
	case SCALAR_VALUE:
		mark_reg_unknown(env, regs, regno);
		return;
	}
		return 0;
	case PTR_TO_BTF_ID:
		mark_reg_known_zero(env, regs, regno);
		regs[regno].type = PTR_TO_BTF_ID | flag;
		regs[regno].btf = btf;
		regs[regno].btf_id = btf_id;
		if (type_may_be_null(flag))
			regs[regno].id = ++env->id_gen;
		return 0;
	case PTR_TO_MEM:
		mark_reg_known_zero(env, regs, regno);
		regs[regno].type = PTR_TO_MEM | flag;
		regs[regno].mem_size = 0;
		return 0;
	default:
		verifier_bug(env, "unexpected reg_type %d in %s\n", reg_type, __func__);
		return -EFAULT;
	}
}
#define DEF_NOT_SUBREG	(0)
@@ -5965,6 +5976,7 @@ static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,
	struct bpf_insn *insn = &env->prog->insnsi[insn_idx];
	int class = BPF_CLASS(insn->code);
	struct bpf_reg_state *val_reg;
	int ret;
	/* Things we already checked for in check_map_access and caller:
	 *  - Reject cases where variable offset may touch kptr
@@ -5998,8 +6010,11 @@ static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,
		/* We can simply mark the value_regno receiving the pointer
		 * value from map as PTR_TO_BTF_ID, with the correct type.
		 */
		mark_btf_ld_reg(env, cur_regs(env), value_regno, PTR_TO_BTF_ID, kptr_field->kptr.btf,
				kptr_field->kptr.btf_id, btf_ld_kptr_type(env, kptr_field));
		ret = mark_btf_ld_reg(env, cur_regs(env), value_regno, PTR_TO_BTF_ID,
				      kptr_field->kptr.btf, kptr_field->kptr.btf_id,
				      btf_ld_kptr_type(env, kptr_field));
		if (ret < 0)
			return ret;
	} else if (class == BPF_STX) {
		val_reg = reg_state(env, value_regno);
		if (!register_is_null(val_reg) &&
@@ -7298,8 +7313,11 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
		clear_trusted_flags(&flag);
	}
	if (atype == BPF_READ && value_regno >= 0)
		mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
	if (atype == BPF_READ && value_regno >= 0) {
		ret = mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
		if (ret < 0)
			return ret;
	}
	return 0;
}
@@ -7353,13 +7371,19 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env,
	/* Simulate access to a PTR_TO_BTF_ID */
	memset(&map_reg, 0, sizeof(map_reg));
	mark_btf_ld_reg(env, &map_reg, 0, PTR_TO_BTF_ID, btf_vmlinux, *map->ops->map_btf_id, 0);
	ret = mark_btf_ld_reg(env, &map_reg, 0, PTR_TO_BTF_ID,
			      btf_vmlinux, *map->ops->map_btf_id, 0);
	if (ret < 0)
		return ret;
	ret = btf_struct_access(&env->log, &map_reg, off, size, atype, &btf_id, &flag, NULL);
	if (ret < 0)
		return ret;
	if (value_regno >= 0)
		mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id, flag);
	if (value_regno >= 0) {
		ret = mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id, flag);
		if (ret < 0)
			return ret;
	}
	return 0;
}
@@ -10413,6 +10437,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
				bpf_log(log, "R%d is not a scalar\n", regno);
				return -EINVAL;
			}
		} else if (arg->arg_type & PTR_UNTRUSTED) {
			/*
			 * Anything is allowed for untrusted arguments, as these are
			 * read-only and probe read instructions would protect against
			 * invalid memory access.
			 */
		} else if (arg->arg_type == ARG_PTR_TO_CTX) {
			ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
			if (ret < 0)
@@ -23122,10 +23152,11 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
				__mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen);
			} else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
				reg->type = PTR_TO_MEM;
				if (arg->arg_type & PTR_MAYBE_NULL)
					reg->type |= PTR_MAYBE_NULL;
				reg->type |= arg->arg_type &
					     (PTR_MAYBE_NULL | PTR_UNTRUSTED | MEM_RDONLY);
				mark_reg_known_zero(env, regs, i);
				reg->mem_size = arg->mem_size;
				if (arg->arg_type & PTR_MAYBE_NULL)
					reg->id = ++env->id_gen;
			} else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
				reg->type = PTR_TO_BTF_ID;
+1 −0
Original line number Diff line number Diff line
@@ -215,6 +215,7 @@ enum libbpf_tristate {
#define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull")))
#define __arg_nullable __attribute((btf_decl_tag("arg:nullable")))
#define __arg_trusted __attribute((btf_decl_tag("arg:trusted")))
#define __arg_untrusted __attribute((btf_decl_tag("arg:untrusted")))
#define __arg_arena __attribute((btf_decl_tag("arg:arena")))

#ifndef ___bpf_concat
+1 −1
Original line number Diff line number Diff line
@@ -72,7 +72,7 @@ static struct {
	{ "new_null_ret", "R0 invalid mem access 'ptr_or_null_'" },
	{ "obj_new_acq", "Unreleased reference id=" },
	{ "use_after_drop", "invalid mem access 'scalar'" },
	{ "ptr_walk_scalar", "type=scalar expected=percpu_ptr_" },
	{ "ptr_walk_scalar", "type=rdonly_untrusted_mem expected=percpu_ptr_" },
	{ "direct_read_lock", "direct access to bpf_spin_lock is disallowed" },
	{ "direct_write_lock", "direct access to bpf_spin_lock is disallowed" },
	{ "direct_read_head", "direct access to bpf_list_head is disallowed" },
Loading