Skip to content

Commit cda0cbf

Browse files
author
Alexei Starovoitov
committed
Merge branch 'bpf-unify-special-map-field-validation-in-verifier'
Mykyta Yatsenko says: ==================== The BPF verifier validates pointers to special map fields (timers, workqueues, task_work) through separate functions that share nearly identical logic. This creates code duplication because of the inconsistent data structure layout in struct bpf_call_arg_meta struct bpf_kfunc_call_arg_meta. This series contains 2 commits: 1. Introduces struct bpf_map_desc to provide a unified representation for map pointer and uid tracking. Previously, bpf_call_arg_meta used separate map_ptr and map_uid fields while bpf_kfunc_call_arg_metaused an anonymous inline struct. This inconsistency made it harder to share validation code between the two paths. 2. Consolidates the validation logic for BPF_TIMER, BPF_WORKQUEUE, and BPF_TASK_WORK field types into a single check_map_field_pointer() function. This eliminates process_wq_func() and process_task_work_func() entirely, and simplifies process_timer_func() to just the PREEMPT_RT check before calling the unified validation. The result is fewer lines of code with clearer structure for future maintenance. Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com> Changes in v2: - Added Signed-off-by to the top commit. - Link to v1: https://lore.kernel.org/r/20260129-verif_special_fields-v1-0-d310b7f146c8@meta.com ==================== Link: https://patch.msgid.link/20260130-verif_special_fields-v2-0-2c59e637da7d@meta.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
2 parents c7fbf8d + f4e72ad commit cda0cbf

1 file changed

Lines changed: 48 additions & 95 deletions

File tree

‎kernel/bpf/verifier.c‎

Lines changed: 48 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,13 @@ static bool bpf_pseudo_kfunc_call(const struct bpf_insn *insn)
272272
insn->src_reg == BPF_PSEUDO_KFUNC_CALL;
273273
}
274274

275+
struct bpf_map_desc {
276+
struct bpf_map *ptr;
277+
int uid;
278+
};
279+
275280
struct bpf_call_arg_meta {
276-
struct bpf_map *map_ptr;
281+
struct bpf_map_desc map;
277282
bool raw_mode;
278283
bool pkt_access;
279284
u8 release_regno;
@@ -283,7 +288,6 @@ struct bpf_call_arg_meta {
283288
u64 msize_max_value;
284289
int ref_obj_id;
285290
int dynptr_id;
286-
int map_uid;
287291
int func_id;
288292
struct btf *btf;
289293
u32 btf_id;
@@ -351,10 +355,7 @@ struct bpf_kfunc_call_arg_meta {
351355
u8 spi;
352356
u8 frameno;
353357
} iter;
354-
struct {
355-
struct bpf_map *ptr;
356-
int uid;
357-
} map;
358+
struct bpf_map_desc map;
358359
u64 mem_size;
359360
};
360361

@@ -8609,7 +8610,8 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno, int flags)
86098610

86108611
/* Check if @regno is a pointer to a specific field in a map value */
86118612
static int check_map_field_pointer(struct bpf_verifier_env *env, u32 regno,
8612-
enum btf_field_type field_type)
8613+
enum btf_field_type field_type,
8614+
struct bpf_map_desc *map_desc)
86138615
{
86148616
struct bpf_reg_state *reg = reg_state(env, regno);
86158617
bool is_const = tnum_is_const(reg->var_off);
@@ -8652,72 +8654,23 @@ static int check_map_field_pointer(struct bpf_verifier_env *env, u32 regno,
86528654
val + reg->off, struct_name, field_off);
86538655
return -EINVAL;
86548656
}
8657+
if (map_desc->ptr) {
8658+
verifier_bug(env, "Two map pointers in a %s helper", struct_name);
8659+
return -EFAULT;
8660+
}
8661+
map_desc->uid = reg->map_uid;
8662+
map_desc->ptr = map;
86558663
return 0;
86568664
}
86578665

86588666
static int process_timer_func(struct bpf_verifier_env *env, int regno,
86598667
struct bpf_call_arg_meta *meta)
86608668
{
8661-
struct bpf_reg_state *reg = reg_state(env, regno);
8662-
struct bpf_map *map = reg->map_ptr;
8663-
int err;
8664-
8665-
err = check_map_field_pointer(env, regno, BPF_TIMER);
8666-
if (err)
8667-
return err;
8668-
8669-
if (meta->map_ptr) {
8670-
verifier_bug(env, "Two map pointers in a timer helper");
8671-
return -EFAULT;
8672-
}
86738669
if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
86748670
verbose(env, "bpf_timer cannot be used for PREEMPT_RT.\n");
86758671
return -EOPNOTSUPP;
86768672
}
8677-
meta->map_uid = reg->map_uid;
8678-
meta->map_ptr = map;
8679-
return 0;
8680-
}
8681-
8682-
static int process_wq_func(struct bpf_verifier_env *env, int regno,
8683-
struct bpf_kfunc_call_arg_meta *meta)
8684-
{
8685-
struct bpf_reg_state *reg = reg_state(env, regno);
8686-
struct bpf_map *map = reg->map_ptr;
8687-
int err;
8688-
8689-
err = check_map_field_pointer(env, regno, BPF_WORKQUEUE);
8690-
if (err)
8691-
return err;
8692-
8693-
if (meta->map.ptr) {
8694-
verifier_bug(env, "Two map pointers in a bpf_wq helper");
8695-
return -EFAULT;
8696-
}
8697-
8698-
meta->map.uid = reg->map_uid;
8699-
meta->map.ptr = map;
8700-
return 0;
8701-
}
8702-
8703-
static int process_task_work_func(struct bpf_verifier_env *env, int regno,
8704-
struct bpf_kfunc_call_arg_meta *meta)
8705-
{
8706-
struct bpf_reg_state *reg = reg_state(env, regno);
8707-
struct bpf_map *map = reg->map_ptr;
8708-
int err;
8709-
8710-
err = check_map_field_pointer(env, regno, BPF_TASK_WORK);
8711-
if (err)
8712-
return err;
8713-
8714-
if (meta->map.ptr) {
8715-
verifier_bug(env, "Two map pointers in a bpf_task_work helper");
8716-
return -EFAULT;
8717-
}
8718-
meta->map.uid = reg->map_uid;
8719-
meta->map.ptr = map;
8720-
return 0;
8673+
return check_map_field_pointer(env, regno, BPF_TIMER, &meta->map);
87218674
}
87228675

87238676
static int process_kptr_func(struct bpf_verifier_env *env, int regno,
@@ -8739,7 +8692,7 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
87398692
return -EINVAL;
87408693
}
87418694
rec = map_ptr->record;
8742-
meta->map_ptr = map_ptr;
8695+
meta->map.ptr = map_ptr;
87438696
}
87448697

87458698
if (!tnum_is_const(reg->var_off)) {
@@ -9246,13 +9199,13 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env,
92469199
const struct bpf_call_arg_meta *meta,
92479200
enum bpf_arg_type *arg_type)
92489201
{
9249-
if (!meta->map_ptr) {
9202+
if (!meta->map.ptr) {
92509203
/* kernel subsystem misconfigured verifier */
92519204
verifier_bug(env, "invalid map_ptr to access map->type");
92529205
return -EFAULT;
92539206
}
92549207

9255-
switch (meta->map_ptr->map_type) {
9208+
switch (meta->map.ptr->map_type) {
92569209
case BPF_MAP_TYPE_SOCKMAP:
92579210
case BPF_MAP_TYPE_SOCKHASH:
92589211
if (*arg_type == ARG_PTR_TO_MAP_VALUE) {
@@ -9906,7 +9859,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
99069859
switch (base_type(arg_type)) {
99079860
case ARG_CONST_MAP_PTR:
99089861
/* bpf_map_xxx(map_ptr) call: remember that map_ptr */
9909-
if (meta->map_ptr) {
9862+
if (meta->map.ptr) {
99109863
/* Use map_uid (which is unique id of inner map) to reject:
99119864
* inner_map1 = bpf_map_lookup_elem(outer_map, key1)
99129865
* inner_map2 = bpf_map_lookup_elem(outer_map, key2)
@@ -9919,23 +9872,23 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
99199872
*
99209873
* Comparing map_ptr is enough to distinguish normal and outer maps.
99219874
*/
9922-
if (meta->map_ptr != reg->map_ptr ||
9923-
meta->map_uid != reg->map_uid) {
9875+
if (meta->map.ptr != reg->map_ptr ||
9876+
meta->map.uid != reg->map_uid) {
99249877
verbose(env,
99259878
"timer pointer in R1 map_uid=%d doesn't match map pointer in R2 map_uid=%d\n",
9926-
meta->map_uid, reg->map_uid);
9879+
meta->map.uid, reg->map_uid);
99279880
return -EINVAL;
99289881
}
99299882
}
9930-
meta->map_ptr = reg->map_ptr;
9931-
meta->map_uid = reg->map_uid;
9883+
meta->map.ptr = reg->map_ptr;
9884+
meta->map.uid = reg->map_uid;
99329885
break;
99339886
case ARG_PTR_TO_MAP_KEY:
99349887
/* bpf_map_xxx(..., map_ptr, ..., key) call:
99359888
* check that [key, key + map->key_size) are within
99369889
* stack limits and initialized
99379890
*/
9938-
if (!meta->map_ptr) {
9891+
if (!meta->map.ptr) {
99399892
/* in function declaration map_ptr must come before
99409893
* map_key, so that it's verified and known before
99419894
* we have to check map_key here. Otherwise it means
@@ -9944,11 +9897,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
99449897
verifier_bug(env, "invalid map_ptr to access map->key");
99459898
return -EFAULT;
99469899
}
9947-
key_size = meta->map_ptr->key_size;
9900+
key_size = meta->map.ptr->key_size;
99489901
err = check_helper_mem_access(env, regno, key_size, BPF_READ, false, NULL);
99499902
if (err)
99509903
return err;
9951-
if (can_elide_value_nullness(meta->map_ptr->map_type)) {
9904+
if (can_elide_value_nullness(meta->map.ptr->map_type)) {
99529905
err = get_constant_map_key(env, reg, key_size, &meta->const_map_key);
99539906
if (err < 0) {
99549907
meta->const_map_key = -1;
@@ -9966,13 +9919,13 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
99669919
/* bpf_map_xxx(..., map_ptr, ..., value) call:
99679920
* check [value, value + map->value_size) validity
99689921
*/
9969-
if (!meta->map_ptr) {
9922+
if (!meta->map.ptr) {
99709923
/* kernel subsystem misconfigured verifier */
99719924
verifier_bug(env, "invalid map_ptr to access map->value");
99729925
return -EFAULT;
99739926
}
99749927
meta->raw_mode = arg_type & MEM_UNINIT;
9975-
err = check_helper_mem_access(env, regno, meta->map_ptr->value_size,
9928+
err = check_helper_mem_access(env, regno, meta->map.ptr->value_size,
99769929
arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
99779930
false, meta);
99789931
break;
@@ -11310,7 +11263,7 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
1131011263
int func_id, int insn_idx)
1131111264
{
1131211265
struct bpf_insn_aux_data *aux = &env->insn_aux_data[insn_idx];
11313-
struct bpf_map *map = meta->map_ptr;
11266+
struct bpf_map *map = meta->map.ptr;
1131411267

1131511268
if (func_id != BPF_FUNC_tail_call &&
1131611269
func_id != BPF_FUNC_map_lookup_elem &&
@@ -11343,11 +11296,11 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
1134311296
}
1134411297

1134511298
if (!aux->map_ptr_state.map_ptr)
11346-
bpf_map_ptr_store(aux, meta->map_ptr,
11347-
!meta->map_ptr->bypass_spec_v1, false);
11348-
else if (aux->map_ptr_state.map_ptr != meta->map_ptr)
11349-
bpf_map_ptr_store(aux, meta->map_ptr,
11350-
!meta->map_ptr->bypass_spec_v1, true);
11299+
bpf_map_ptr_store(aux, meta->map.ptr,
11300+
!meta->map.ptr->bypass_spec_v1, false);
11301+
else if (aux->map_ptr_state.map_ptr != meta->map.ptr)
11302+
bpf_map_ptr_store(aux, meta->map.ptr,
11303+
!meta->map.ptr->bypass_spec_v1, true);
1135111304
return 0;
1135211305
}
1135311306

@@ -11357,7 +11310,7 @@ record_func_key(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
1135711310
{
1135811311
struct bpf_insn_aux_data *aux = &env->insn_aux_data[insn_idx];
1135911312
struct bpf_reg_state *reg;
11360-
struct bpf_map *map = meta->map_ptr;
11313+
struct bpf_map *map = meta->map.ptr;
1136111314
u64 val, max;
1136211315
int err;
1136311316

@@ -11913,22 +11866,22 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
1191311866
* can check 'value_size' boundary of memory access
1191411867
* to map element returned from bpf_map_lookup_elem()
1191511868
*/
11916-
if (meta.map_ptr == NULL) {
11869+
if (meta.map.ptr == NULL) {
1191711870
verifier_bug(env, "unexpected null map_ptr");
1191811871
return -EFAULT;
1191911872
}
1192011873

1192111874
if (func_id == BPF_FUNC_map_lookup_elem &&
11922-
can_elide_value_nullness(meta.map_ptr->map_type) &&
11875+
can_elide_value_nullness(meta.map.ptr->map_type) &&
1192311876
meta.const_map_key >= 0 &&
11924-
meta.const_map_key < meta.map_ptr->max_entries)
11877+
meta.const_map_key < meta.map.ptr->max_entries)
1192511878
ret_flag &= ~PTR_MAYBE_NULL;
1192611879

11927-
regs[BPF_REG_0].map_ptr = meta.map_ptr;
11928-
regs[BPF_REG_0].map_uid = meta.map_uid;
11880+
regs[BPF_REG_0].map_ptr = meta.map.ptr;
11881+
regs[BPF_REG_0].map_uid = meta.map.uid;
1192911882
regs[BPF_REG_0].type = PTR_TO_MAP_VALUE | ret_flag;
1193011883
if (!type_may_be_null(ret_flag) &&
11931-
btf_record_has_field(meta.map_ptr->record, BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK)) {
11884+
btf_record_has_field(meta.map.ptr->record, BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK)) {
1193211885
regs[BPF_REG_0].id = ++env->id_gen;
1193311886
}
1193411887
break;
@@ -12031,7 +11984,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
1203111984
if (type_may_be_null(regs[BPF_REG_0].type))
1203211985
regs[BPF_REG_0].id = ++env->id_gen;
1203311986

12034-
if (helper_multiple_ref_obj_use(func_id, meta.map_ptr)) {
11987+
if (helper_multiple_ref_obj_use(func_id, meta.map.ptr)) {
1203511988
verifier_bug(env, "func %s#%d sets ref_obj_id more than once",
1203611989
func_id_name(func_id), func_id);
1203711990
return -EFAULT;
@@ -12043,7 +11996,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
1204311996
if (is_ptr_cast_function(func_id) || is_dynptr_ref_function(func_id)) {
1204411997
/* For release_reference() */
1204511998
regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
12046-
} else if (is_acquire_function(func_id, meta.map_ptr)) {
11999+
} else if (is_acquire_function(func_id, meta.map.ptr)) {
1204712000
int id = acquire_reference(env, insn_idx);
1204812001

1204912002
if (id < 0)
@@ -12058,7 +12011,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
1205812011
if (err)
1205912012
return err;
1206012013

12061-
err = check_map_func_compatibility(env, meta.map_ptr, func_id);
12014+
err = check_map_func_compatibility(env, meta.map.ptr, func_id);
1206212015
if (err)
1206312016
return err;
1206412017

@@ -13753,7 +13706,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
1375313706
verbose(env, "arg#%d doesn't point to a map value\n", i);
1375413707
return -EINVAL;
1375513708
}
13756-
ret = process_wq_func(env, regno, meta);
13709+
ret = check_map_field_pointer(env, regno, BPF_WORKQUEUE, &meta->map);
1375713710
if (ret < 0)
1375813711
return ret;
1375913712
break;
@@ -13762,7 +13715,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
1376213715
verbose(env, "arg#%d doesn't point to a map value\n", i);
1376313716
return -EINVAL;
1376413717
}
13765-
ret = process_task_work_func(env, regno, meta);
13718+
ret = check_map_field_pointer(env, regno, BPF_TASK_WORK, &meta->map);
1376613719
if (ret < 0)
1376713720
return ret;
1376813721
break;

0 commit comments

Comments
 (0)