diff mbox series

[RFC,2/3] bpf: Enforce granted permissions in a map fd at verifier level

Message ID 20220926154430.1552800-3-roberto.sassu@huaweicloud.com
State New
Headers show
Series bpf: Enforce map fd modes in verifier | expand

Commit Message

Roberto Sassu Sept. 26, 2022, 3:44 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

Commit afdb09c720b62 ("security: bpf: Add LSM hooks for bpf object related
syscall") introduced new eBPF-related hooks in the LSM framework, for
programs and maps, aiming at enforcing permissions per eBPF object.

Commit 6e71b04a82248 ("bpf: Add file mode configuration into bpf maps")
further introduced the BPF_F_RDONLY and BPF_F_WRONLY flags, for user space
to request specific permissions when using a given eBPF object.

The two patches are related, as the first ensures that LSMs grant to user
space the requested permissions (read and/or write) for performing
operations on an eBPF object. The second ensures that the granted
permissions are sufficient to perform a requested operation.

While the second check has been added for operations of the bpf() system
call that directly deal with a map, such as BPF_MAP_*_ELEM, it is missing
for bpf() system call operations that still receive a map fd, but modify a
map indirectly: map iterators (addressed separately) and the eBPF verifier.

An eBPF program might contain a map fd as argument of the BPF_PSEUDO_MAP_FD
and BPF_PSEUDO_MAP_IDX instructions. The eBPF verifier processes those
instructions, and replaces the map fd with the corresponding map address,
which can be then passed to eBPF helpers, such as bpf_map_lookup_elem() and
bpf_map_update_elem(). This has the same effect of invoking the bpf()
system call and executing the BPF_MAP_*_ELEM operations.

The problem is that, unlike BPF_MAP_*_ELEM operations of the bpf() system
call, the eBPF verifier does not check the fd modes before letting the eBPF
program do map operations. As a consequence, for example, a read-only fd
can be provided to the eBPF program, allowing it to do a map update.

A different behavior occurs when the map flags BPF_F_RDONLY_PROG and
BPF_F_WRONLY_PROG are set at map creation time. Commit 591fe9888d78 ("bpf:
add program side {rd, wr}only support for maps") ensures that only the map
operations compatible with the map flags can be executed by the eBPF
program, otherwise the verifier refuses to run that program.

As the verifier can already restrict map operations, rely on the same
mechanism to enforce permissions given with the fd. Providing a read-only
fd has the same effect of setting the BPF_F_RDONLY_PROG map flag, except
that the effect is limited to the program execution and not for the map
lifetime.

If multiple map fds are provided to the eBPF program, combine the fd modes,
as the verifier is not able to track the exact fd a map address has been
obtained from.

Finally, make sure that the resulting fd modes don't give to the eBPF
program more permissions than the ones granted by map flags. Instead, given
the initial permissions granted by map flags, clear the ones that are
missing from the fd.

Although normally map fd-based operations are not affected by BPF_F_*_PROG,
in this case it cannot be, as it is the eBPF program itself doing map
operations, which is what BPF_F_*_PROG are designed to restrict.

Cc: stable@vger.kernel.org
Fixes: 6e71b04a82248 ("bpf: Add file mode configuration into bpf maps")
Reported by: Lorenz Bauer <oss@lmb.io>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/bpf.h          | 13 +++++++++++++
 include/linux/bpf_verifier.h |  1 +
 kernel/bpf/verifier.c        | 26 ++++++++++++++++++++++++--
 3 files changed, 38 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index edd43edb27d6..1e18f11df7ca 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1415,6 +1415,19 @@  static inline u32 bpf_map_flags_to_cap(struct bpf_map *map)
 		return BPF_MAP_CAN_READ | BPF_MAP_CAN_WRITE;
 }
 
+static inline u32 bpf_fd_modes_to_cap(fmode_t mode)
+{
+	u32 cap = 0;
+
+	if (mode & FMODE_CAN_READ)
+		cap |= BPF_MAP_CAN_READ;
+
+	if (mode & FMODE_CAN_WRITE)
+		cap |= BPF_MAP_CAN_WRITE;
+
+	return cap;
+}
+
 static inline bool bpf_map_flags_access_ok(u32 access_flags)
 {
 	return (access_flags & (BPF_F_RDONLY_PROG | BPF_F_WRONLY_PROG)) !=
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 9e1e6965f407..3f490bae0bcd 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -501,6 +501,7 @@  struct bpf_verifier_env {
 	struct bpf_verifier_state_list **explored_states; /* search pruning optimization */
 	struct bpf_verifier_state_list *free_list;
 	struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */
+	u32 used_maps_caps[MAX_USED_MAPS]; /* array of map capabilities possessed by eBPF program */
 	struct btf_mod_pair used_btfs[MAX_USED_BTFS]; /* array of BTF's used by BPF program */
 	u32 used_map_cnt;		/* number of used maps */
 	u32 used_btf_cnt;		/* number of used BTF objects */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6f6d2d511c06..ac9bd4402169 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3531,6 +3531,14 @@  static int check_map_access_type(struct bpf_verifier_env *env, u32 regno,
 	struct bpf_reg_state *regs = cur_regs(env);
 	struct bpf_map *map = regs[regno].map_ptr;
 	u32 cap = bpf_map_flags_to_cap(map);
+	int i;
+
+	for (i = 0; i < env->used_map_cnt; i++) {
+		if (env->used_maps[i] == map) {
+			cap &= env->used_maps_caps[i];
+			break;
+		}
+	}
 
 	if (type == BPF_WRITE && !(cap & BPF_MAP_CAN_WRITE)) {
 		verbose(env, "write into map forbidden, value_size=%d off=%d size=%d\n",
@@ -7040,6 +7048,8 @@  record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 {
 	struct bpf_insn_aux_data *aux = &env->insn_aux_data[insn_idx];
 	struct bpf_map *map = meta->map_ptr;
+	u32 cap;
+	int i;
 
 	if (func_id != BPF_FUNC_tail_call &&
 	    func_id != BPF_FUNC_map_lookup_elem &&
@@ -7058,11 +7068,20 @@  record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 		return -EINVAL;
 	}
 
+	cap = bpf_map_flags_to_cap(map);
+
+	for (i = 0; i < env->used_map_cnt; i++) {
+		if (env->used_maps[i] == map) {
+			cap &= env->used_maps_caps[i];
+			break;
+		}
+	}
+
 	/* In case of read-only, some additional restrictions
 	 * need to be applied in order to prevent altering the
 	 * state of the map from program side.
 	 */
-	if ((map->map_flags & BPF_F_RDONLY_PROG) &&
+	if (!(cap & BPF_MAP_CAN_WRITE) &&
 	    (func_id == BPF_FUNC_map_delete_elem ||
 	     func_id == BPF_FUNC_map_update_elem ||
 	     func_id == BPF_FUNC_map_push_elem ||
@@ -12870,6 +12889,7 @@  static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 			/* check whether we recorded this map already */
 			for (j = 0; j < env->used_map_cnt; j++) {
 				if (env->used_maps[j] == map) {
+					env->used_maps_caps[j] |= bpf_fd_modes_to_cap(f.file->f_mode);
 					aux->map_index = j;
 					fdput(f);
 					goto next_insn;
@@ -12889,7 +12909,9 @@  static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 			bpf_map_inc(map);
 
 			aux->map_index = env->used_map_cnt;
-			env->used_maps[env->used_map_cnt++] = map;
+			env->used_maps[env->used_map_cnt] = map;
+			env->used_maps_caps[env->used_map_cnt] = bpf_fd_modes_to_cap(f.file->f_mode);
+			env->used_map_cnt++;
 
 			if (bpf_map_is_cgroup_storage(map) &&
 			    bpf_cgroup_storage_assign(env->prog->aux, map)) {