Skip to content

Commit 535ead4

Browse files
committed
bpf: factor out fetching bpf_map from FD and adding it to used_maps list
Factor out the logic to extract bpf_map instances from FD embedded in bpf_insns, adding it to the list of used_maps (unless it's already there, in which case we just reuse map's index). This simplifies the logic in resolve_pseudo_ldimm64(), especially around `struct fd` handling, as all that is now neatly contained in the helper and doesn't leak into a dozen error handling paths. Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
1 parent 51a1ca9 commit 535ead4

File tree

1 file changed

+66
-49
lines changed

1 file changed

+66
-49
lines changed

kernel/bpf/verifier.c

Lines changed: 66 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18865,6 +18865,58 @@ static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
1886518865
map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
1886618866
}
1886718867

18868+
/* Add map behind fd to used maps list, if it's not already there, and return
18869+
* its index. Also set *reused to true if this map was already in the list of
18870+
* used maps.
18871+
* Returns <0 on error, or >= 0 index, on success.
18872+
*/
18873+
static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reused)
18874+
{
18875+
struct fd f = fdget(fd);
18876+
struct bpf_map *map;
18877+
int i;
18878+
18879+
map = __bpf_map_get(f);
18880+
if (IS_ERR(map)) {
18881+
verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
18882+
return PTR_ERR(map);
18883+
}
18884+
18885+
/* check whether we recorded this map already */
18886+
for (i = 0; i < env->used_map_cnt; i++) {
18887+
if (env->used_maps[i] == map) {
18888+
*reused = true;
18889+
fdput(f);
18890+
return i;
18891+
}
18892+
}
18893+
18894+
if (env->used_map_cnt >= MAX_USED_MAPS) {
18895+
verbose(env, "The total number of maps per program has reached the limit of %u\n",
18896+
MAX_USED_MAPS);
18897+
fdput(f);
18898+
return -E2BIG;
18899+
}
18900+
18901+
if (env->prog->sleepable)
18902+
atomic64_inc(&map->sleepable_refcnt);
18903+
18904+
/* hold the map. If the program is rejected by verifier,
18905+
* the map will be released by release_maps() or it
18906+
* will be used by the valid program until it's unloaded
18907+
* and all maps are released in bpf_free_used_maps()
18908+
*/
18909+
bpf_map_inc(map);
18910+
18911+
*reused = false;
18912+
env->used_maps[env->used_map_cnt++] = map;
18913+
18914+
fdput(f);
18915+
18916+
return env->used_map_cnt - 1;
18917+
18918+
}
18919+
1886818920
/* find and rewrite pseudo imm in ld_imm64 instructions:
1886918921
*
1887018922
* 1. if it accesses map FD, replace it with actual map pointer.
@@ -18876,7 +18928,7 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
1887618928
{
1887718929
struct bpf_insn *insn = env->prog->insnsi;
1887818930
int insn_cnt = env->prog->len;
18879-
int i, j, err;
18931+
int i, err;
1888018932

1888118933
err = bpf_prog_calc_tag(env->prog);
1888218934
if (err)
@@ -18893,9 +18945,10 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
1889318945
if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {
1889418946
struct bpf_insn_aux_data *aux;
1889518947
struct bpf_map *map;
18896-
struct fd f;
18948+
int map_idx;
1889718949
u64 addr;
1889818950
u32 fd;
18951+
bool reused;
1889918952

1890018953
if (i == insn_cnt - 1 || insn[1].code != 0 ||
1890118954
insn[1].dst_reg != 0 || insn[1].src_reg != 0 ||
@@ -18956,20 +19009,18 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
1895619009
break;
1895719010
}
1895819011

18959-
f = fdget(fd);
18960-
map = __bpf_map_get(f);
18961-
if (IS_ERR(map)) {
18962-
verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
18963-
return PTR_ERR(map);
18964-
}
19012+
map_idx = add_used_map_from_fd(env, fd, &reused);
19013+
if (map_idx < 0)
19014+
return map_idx;
19015+
map = env->used_maps[map_idx];
19016+
19017+
aux = &env->insn_aux_data[i];
19018+
aux->map_index = map_idx;
1896519019

1896619020
err = check_map_prog_compatibility(env, map, env->prog);
18967-
if (err) {
18968-
fdput(f);
19021+
if (err)
1896919022
return err;
18970-
}
1897119023

18972-
aux = &env->insn_aux_data[i];
1897319024
if (insn[0].src_reg == BPF_PSEUDO_MAP_FD ||
1897419025
insn[0].src_reg == BPF_PSEUDO_MAP_IDX) {
1897519026
addr = (unsigned long)map;
@@ -18978,21 +19029,18 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
1897819029

1897919030
if (off >= BPF_MAX_VAR_OFF) {
1898019031
verbose(env, "direct value offset of %u is not allowed\n", off);
18981-
fdput(f);
1898219032
return -EINVAL;
1898319033
}
1898419034

1898519035
if (!map->ops->map_direct_value_addr) {
1898619036
verbose(env, "no direct value access support for this map type\n");
18987-
fdput(f);
1898819037
return -EINVAL;
1898919038
}
1899019039

1899119040
err = map->ops->map_direct_value_addr(map, &addr, off);
1899219041
if (err) {
1899319042
verbose(env, "invalid access to map value pointer, value_size=%u off=%u\n",
1899419043
map->value_size, off);
18995-
fdput(f);
1899619044
return err;
1899719045
}
1899819046

@@ -19003,70 +19051,39 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
1900319051
insn[0].imm = (u32)addr;
1900419052
insn[1].imm = addr >> 32;
1900519053

19006-
/* check whether we recorded this map already */
19007-
for (j = 0; j < env->used_map_cnt; j++) {
19008-
if (env->used_maps[j] == map) {
19009-
aux->map_index = j;
19010-
fdput(f);
19011-
goto next_insn;
19012-
}
19013-
}
19014-
19015-
if (env->used_map_cnt >= MAX_USED_MAPS) {
19016-
verbose(env, "The total number of maps per program has reached the limit of %u\n",
19017-
MAX_USED_MAPS);
19018-
fdput(f);
19019-
return -E2BIG;
19020-
}
19021-
19022-
if (env->prog->sleepable)
19023-
atomic64_inc(&map->sleepable_refcnt);
19024-
/* hold the map. If the program is rejected by verifier,
19025-
* the map will be released by release_maps() or it
19026-
* will be used by the valid program until it's unloaded
19027-
* and all maps are released in bpf_free_used_maps()
19028-
*/
19029-
bpf_map_inc(map);
19030-
19031-
aux->map_index = env->used_map_cnt;
19032-
env->used_maps[env->used_map_cnt++] = map;
19054+
/* proceed with extra checks only if its newly added used map */
19055+
if (reused)
19056+
goto next_insn;
1903319057

1903419058
if (bpf_map_is_cgroup_storage(map) &&
1903519059
bpf_cgroup_storage_assign(env->prog->aux, map)) {
1903619060
verbose(env, "only one cgroup storage of each type is allowed\n");
19037-
fdput(f);
1903819061
return -EBUSY;
1903919062
}
1904019063
if (map->map_type == BPF_MAP_TYPE_ARENA) {
1904119064
if (env->prog->aux->arena) {
1904219065
verbose(env, "Only one arena per program\n");
19043-
fdput(f);
1904419066
return -EBUSY;
1904519067
}
1904619068
if (!env->allow_ptr_leaks || !env->bpf_capable) {
1904719069
verbose(env, "CAP_BPF and CAP_PERFMON are required to use arena\n");
19048-
fdput(f);
1904919070
return -EPERM;
1905019071
}
1905119072
if (!env->prog->jit_requested) {
1905219073
verbose(env, "JIT is required to use arena\n");
19053-
fdput(f);
1905419074
return -EOPNOTSUPP;
1905519075
}
1905619076
if (!bpf_jit_supports_arena()) {
1905719077
verbose(env, "JIT doesn't support arena\n");
19058-
fdput(f);
1905919078
return -EOPNOTSUPP;
1906019079
}
1906119080
env->prog->aux->arena = (void *)map;
1906219081
if (!bpf_arena_get_user_vm_start(env->prog->aux->arena)) {
1906319082
verbose(env, "arena's user address must be set via map_extra or mmap()\n");
19064-
fdput(f);
1906519083
return -EINVAL;
1906619084
}
1906719085
}
1906819086

19069-
fdput(f);
1907019087
next_insn:
1907119088
insn++;
1907219089
i++;

0 commit comments

Comments
 (0)