Skip to content

Commit 942b8b3

Browse files
committed
Daniel Borkmann says: ==================== pull-request: bpf 2023-11-08 We've added 16 non-merge commits during the last 6 day(s) which contain a total of 30 files changed, 341 insertions(+), 130 deletions(-). The main changes are: 1) Fix a BPF verifier issue in precision tracking for BPF_ALU | BPF_TO_BE | BPF_END where the source register was incorrectly marked as precise, from Shung-Hsi Yu. 2) Fix a concurrency issue in bpf_timer where the former could still have been alive after an application releases or unpins the map, from Hou Tao. 3) Fix a BPF verifier issue where immediates are incorrectly cast to u32 before being spilled and therefore losing sign information, from Hao Sun. 4) Fix a misplaced BPF_TRACE_ITER in check_css_task_iter_allowlist which incorrectly compared bpf_prog_type with bpf_attach_type, from Chuyi Zhou. 5) Add __bpf_hook_{start,end} as well as __bpf_kfunc_{start,end}_defs macros, migrate all BPF-related __diag callsites over to it, and add a new __diag_ignore_all for -Wmissing-declarations to the macros to address recent build warnings, from Dave Marchevsky. 6) Fix broken BPF selftest build of xdp_hw_metadata test on architectures where char is not signed, from Björn Töpel. 7) Fix test_maps selftest to properly use LIBBPF_OPTS() macro to initialize the bpf_map_create_opts, from Andrii Nakryiko. 8) Fix bpffs selftest to avoid unmounting /sys/kernel/debug as it may have been mounted and used by other applications already, from Manu Bretelle. 9) Fix a build issue without CONFIG_CGROUPS wrt css_task open-coded iterators, from Matthieu Baerts. * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: selftests/bpf: get trusted cgrp from bpf_iter__cgroup directly bpf: Let verifier consider {task,cgroup} is trusted in bpf_iter_reg selftests/bpf: Fix broken build where char is unsigned selftests/bpf: precision tracking test for BPF_NEG and BPF_END bpf: Fix precision tracking for BPF_ALU | BPF_TO_BE | BPF_END selftests/bpf: Add test for using css_task iter in sleepable progs selftests/bpf: Add tests for css_task iter combining with cgroup iter bpf: Relax allowlist for css_task iter selftests/bpf: fix test_maps' use of bpf_map_create_opts bpf: Check map->usercnt after timer->timer is assigned bpf: Add __bpf_hook_{start,end} macros bpf: Add __bpf_kfunc_{start,end}_defs macros selftests/bpf: fix test_bpffs selftests/bpf: Add test for immediate spilled to stack bpf: Fix check_stack_write_fixed_off() to correctly spill imm bpf: fix compilation error without CGROUPS ==================== Link: https://lore.kernel.org/r/20231108132448.1970-1-daniel@iogearbox.net Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 9bc64bd + 8e1b802 commit 942b8b3

File tree

30 files changed

+341
-130
lines changed

30 files changed

+341
-130
lines changed

Documentation/bpf/kfuncs.rst

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,14 @@ prototype in a header for the wrapper kfunc.
3737
An example is given below::
3838

3939
/* Disables missing prototype warnings */
40-
__diag_push();
41-
__diag_ignore_all("-Wmissing-prototypes",
42-
"Global kfuncs as their definitions will be in BTF");
40+
__bpf_kfunc_start_defs();
4341

4442
__bpf_kfunc struct task_struct *bpf_find_get_task_by_vpid(pid_t nr)
4543
{
4644
return find_get_task_by_vpid(nr);
4745
}
4846

49-
__diag_pop();
47+
__bpf_kfunc_end_defs();
5048

5149
A wrapper kfunc is often needed when we need to annotate parameters of the
5250
kfunc. Otherwise one may directly make the kfunc visible to the BPF program by

include/linux/btf.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@
8484
*/
8585
#define __bpf_kfunc __used noinline
8686

87+
#define __bpf_kfunc_start_defs() \
88+
__diag_push(); \
89+
__diag_ignore_all("-Wmissing-declarations", \
90+
"Global kfuncs as their definitions will be in BTF");\
91+
__diag_ignore_all("-Wmissing-prototypes", \
92+
"Global kfuncs as their definitions will be in BTF")
93+
94+
#define __bpf_kfunc_end_defs() __diag_pop()
95+
#define __bpf_hook_start() __bpf_kfunc_start_defs()
96+
#define __bpf_hook_end() __bpf_kfunc_end_defs()
97+
8798
/*
8899
* Return the name of the passed struct, if exists, or halt the build if for
89100
* example the structure gets renamed. In this way, developers have to revisit

kernel/bpf/bpf_iter.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -782,9 +782,7 @@ struct bpf_iter_num_kern {
782782
int end; /* final value, exclusive */
783783
} __aligned(8);
784784

785-
__diag_push();
786-
__diag_ignore_all("-Wmissing-prototypes",
787-
"Global functions as their definitions will be in vmlinux BTF");
785+
__bpf_kfunc_start_defs();
788786

789787
__bpf_kfunc int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end)
790788
{
@@ -843,4 +841,4 @@ __bpf_kfunc void bpf_iter_num_destroy(struct bpf_iter_num *it)
843841
s->cur = s->end = 0;
844842
}
845843

846-
__diag_pop();
844+
__bpf_kfunc_end_defs();

kernel/bpf/cgroup_iter.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ static struct bpf_iter_reg bpf_cgroup_reg_info = {
282282
.ctx_arg_info_size = 1,
283283
.ctx_arg_info = {
284284
{ offsetof(struct bpf_iter__cgroup, cgroup),
285-
PTR_TO_BTF_ID_OR_NULL },
285+
PTR_TO_BTF_ID_OR_NULL | PTR_TRUSTED },
286286
},
287287
.seq_info = &cgroup_iter_seq_info,
288288
};
@@ -305,9 +305,7 @@ struct bpf_iter_css_kern {
305305
unsigned int flags;
306306
} __attribute__((aligned(8)));
307307

308-
__diag_push();
309-
__diag_ignore_all("-Wmissing-prototypes",
310-
"Global functions as their definitions will be in vmlinux BTF");
308+
__bpf_kfunc_start_defs();
311309

312310
__bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
313311
struct cgroup_subsys_state *start, unsigned int flags)
@@ -358,4 +356,4 @@ __bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
358356
{
359357
}
360358

361-
__diag_pop();
359+
__bpf_kfunc_end_defs();

kernel/bpf/cpumask.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ static bool cpu_valid(u32 cpu)
3434
return cpu < nr_cpu_ids;
3535
}
3636

37-
__diag_push();
38-
__diag_ignore_all("-Wmissing-prototypes",
39-
"Global kfuncs as their definitions will be in BTF");
37+
__bpf_kfunc_start_defs();
4038

4139
/**
4240
* bpf_cpumask_create() - Create a mutable BPF cpumask.
@@ -407,7 +405,7 @@ __bpf_kfunc u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
407405
return cpumask_any_and_distribute(src1, src2);
408406
}
409407

410-
__diag_pop();
408+
__bpf_kfunc_end_defs();
411409

412410
BTF_SET8_START(cpumask_kfunc_btf_ids)
413411
BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL)

kernel/bpf/helpers.c

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,13 +1177,6 @@ BPF_CALL_3(bpf_timer_init, struct bpf_timer_kern *, timer, struct bpf_map *, map
11771177
ret = -EBUSY;
11781178
goto out;
11791179
}
1180-
if (!atomic64_read(&map->usercnt)) {
1181-
/* maps with timers must be either held by user space
1182-
* or pinned in bpffs.
1183-
*/
1184-
ret = -EPERM;
1185-
goto out;
1186-
}
11871180
/* allocate hrtimer via map_kmalloc to use memcg accounting */
11881181
t = bpf_map_kmalloc_node(map, sizeof(*t), GFP_ATOMIC, map->numa_node);
11891182
if (!t) {
@@ -1196,7 +1189,21 @@ BPF_CALL_3(bpf_timer_init, struct bpf_timer_kern *, timer, struct bpf_map *, map
11961189
rcu_assign_pointer(t->callback_fn, NULL);
11971190
hrtimer_init(&t->timer, clockid, HRTIMER_MODE_REL_SOFT);
11981191
t->timer.function = bpf_timer_cb;
1199-
timer->timer = t;
1192+
WRITE_ONCE(timer->timer, t);
1193+
/* Guarantee the order between timer->timer and map->usercnt. So
1194+
* when there are concurrent uref release and bpf timer init, either
1195+
* bpf_timer_cancel_and_free() called by uref release reads a no-NULL
1196+
* timer or atomic64_read() below returns a zero usercnt.
1197+
*/
1198+
smp_mb();
1199+
if (!atomic64_read(&map->usercnt)) {
1200+
/* maps with timers must be either held by user space
1201+
* or pinned in bpffs.
1202+
*/
1203+
WRITE_ONCE(timer->timer, NULL);
1204+
kfree(t);
1205+
ret = -EPERM;
1206+
}
12001207
out:
12011208
__bpf_spin_unlock_irqrestore(&timer->lock);
12021209
return ret;
@@ -1374,7 +1381,7 @@ void bpf_timer_cancel_and_free(void *val)
13741381
/* The subsequent bpf_timer_start/cancel() helpers won't be able to use
13751382
* this timer, since it won't be initialized.
13761383
*/
1377-
timer->timer = NULL;
1384+
WRITE_ONCE(timer->timer, NULL);
13781385
out:
13791386
__bpf_spin_unlock_irqrestore(&timer->lock);
13801387
if (!t)
@@ -1886,9 +1893,7 @@ void bpf_rb_root_free(const struct btf_field *field, void *rb_root,
18861893
}
18871894
}
18881895

1889-
__diag_push();
1890-
__diag_ignore_all("-Wmissing-prototypes",
1891-
"Global functions as their definitions will be in vmlinux BTF");
1896+
__bpf_kfunc_start_defs();
18921897

18931898
__bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
18941899
{
@@ -2505,7 +2510,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
25052510
WARN(1, "A call to BPF exception callback should never return\n");
25062511
}
25072512

2508-
__diag_pop();
2513+
__bpf_kfunc_end_defs();
25092514

25102515
BTF_SET8_START(generic_btf_ids)
25112516
#ifdef CONFIG_KEXEC_CORE
@@ -2564,15 +2569,17 @@ BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
25642569
BTF_ID_FLAGS(func, bpf_iter_task_vma_new, KF_ITER_NEW | KF_RCU)
25652570
BTF_ID_FLAGS(func, bpf_iter_task_vma_next, KF_ITER_NEXT | KF_RET_NULL)
25662571
BTF_ID_FLAGS(func, bpf_iter_task_vma_destroy, KF_ITER_DESTROY)
2572+
#ifdef CONFIG_CGROUPS
25672573
BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
25682574
BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
25692575
BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
2570-
BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
2571-
BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
2572-
BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
25732576
BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
25742577
BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
25752578
BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
2579+
#endif
2580+
BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
2581+
BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
2582+
BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
25762583
BTF_ID_FLAGS(func, bpf_dynptr_adjust)
25772584
BTF_ID_FLAGS(func, bpf_dynptr_is_null)
25782585
BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)

kernel/bpf/map_iter.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,7 @@ static int __init bpf_map_iter_init(void)
193193

194194
late_initcall(bpf_map_iter_init);
195195

196-
__diag_push();
197-
__diag_ignore_all("-Wmissing-prototypes",
198-
"Global functions as their definitions will be in vmlinux BTF");
196+
__bpf_kfunc_start_defs();
199197

200198
__bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
201199
{
@@ -213,7 +211,7 @@ __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
213211
return ret;
214212
}
215213

216-
__diag_pop();
214+
__bpf_kfunc_end_defs();
217215

218216
BTF_SET8_START(bpf_map_iter_kfunc_ids)
219217
BTF_ID_FLAGS(func, bpf_map_sum_elem_count, KF_TRUSTED_ARGS)

kernel/bpf/task_iter.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ static struct bpf_iter_reg task_reg_info = {
704704
.ctx_arg_info_size = 1,
705705
.ctx_arg_info = {
706706
{ offsetof(struct bpf_iter__task, task),
707-
PTR_TO_BTF_ID_OR_NULL },
707+
PTR_TO_BTF_ID_OR_NULL | PTR_TRUSTED },
708708
},
709709
.seq_info = &task_seq_info,
710710
.fill_link_info = bpf_iter_fill_link_info,
@@ -822,9 +822,7 @@ struct bpf_iter_task_vma_kern {
822822
struct bpf_iter_task_vma_kern_data *data;
823823
} __attribute__((aligned(8)));
824824

825-
__diag_push();
826-
__diag_ignore_all("-Wmissing-prototypes",
827-
"Global functions as their definitions will be in vmlinux BTF");
825+
__bpf_kfunc_start_defs();
828826

829827
__bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
830828
struct task_struct *task, u64 addr)
@@ -890,7 +888,9 @@ __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
890888
}
891889
}
892890

893-
__diag_pop();
891+
__bpf_kfunc_end_defs();
892+
893+
#ifdef CONFIG_CGROUPS
894894

895895
struct bpf_iter_css_task {
896896
__u64 __opaque[1];
@@ -900,9 +900,7 @@ struct bpf_iter_css_task_kern {
900900
struct css_task_iter *css_it;
901901
} __attribute__((aligned(8)));
902902

903-
__diag_push();
904-
__diag_ignore_all("-Wmissing-prototypes",
905-
"Global functions as their definitions will be in vmlinux BTF");
903+
__bpf_kfunc_start_defs();
906904

907905
__bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
908906
struct cgroup_subsys_state *css, unsigned int flags)
@@ -948,7 +946,9 @@ __bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
948946
bpf_mem_free(&bpf_global_ma, kit->css_it);
949947
}
950948

951-
__diag_pop();
949+
__bpf_kfunc_end_defs();
950+
951+
#endif /* CONFIG_CGROUPS */
952952

953953
struct bpf_iter_task {
954954
__u64 __opaque[3];
@@ -969,9 +969,7 @@ enum {
969969
BPF_TASK_ITER_PROC_THREADS
970970
};
971971

972-
__diag_push();
973-
__diag_ignore_all("-Wmissing-prototypes",
974-
"Global functions as their definitions will be in vmlinux BTF");
972+
__bpf_kfunc_start_defs();
975973

976974
__bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
977975
struct task_struct *task__nullable, unsigned int flags)
@@ -1041,7 +1039,7 @@ __bpf_kfunc void bpf_iter_task_destroy(struct bpf_iter_task *it)
10411039
{
10421040
}
10431041

1044-
__diag_pop();
1042+
__bpf_kfunc_end_defs();
10451043

10461044
DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
10471045

kernel/bpf/verifier.c

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3742,7 +3742,12 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
37423742
if (class == BPF_ALU || class == BPF_ALU64) {
37433743
if (!bt_is_reg_set(bt, dreg))
37443744
return 0;
3745-
if (opcode == BPF_MOV) {
3745+
if (opcode == BPF_END || opcode == BPF_NEG) {
3746+
/* sreg is reserved and unused
3747+
* dreg still need precision before this insn
3748+
*/
3749+
return 0;
3750+
} else if (opcode == BPF_MOV) {
37463751
if (BPF_SRC(insn->code) == BPF_X) {
37473752
/* dreg = sreg or dreg = (s8, s16, s32)sreg
37483753
* dreg needs precision after this insn
@@ -4674,7 +4679,7 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
46744679
insn->imm != 0 && env->bpf_capable) {
46754680
struct bpf_reg_state fake_reg = {};
46764681

4677-
__mark_reg_known(&fake_reg, (u32)insn->imm);
4682+
__mark_reg_known(&fake_reg, insn->imm);
46784683
fake_reg.type = SCALAR_VALUE;
46794684
save_register_state(state, spi, &fake_reg, size);
46804685
} else if (reg && is_spillable_regtype(reg->type)) {
@@ -5388,7 +5393,9 @@ static bool in_rcu_cs(struct bpf_verifier_env *env)
53885393
/* Once GCC supports btf_type_tag the following mechanism will be replaced with tag check */
53895394
BTF_SET_START(rcu_protected_types)
53905395
BTF_ID(struct, prog_test_ref_kfunc)
5396+
#ifdef CONFIG_CGROUPS
53915397
BTF_ID(struct, cgroup)
5398+
#endif
53925399
BTF_ID(struct, bpf_cpumask)
53935400
BTF_ID(struct, task_struct)
53945401
BTF_SET_END(rcu_protected_types)
@@ -10835,7 +10842,9 @@ BTF_ID(func, bpf_dynptr_clone)
1083510842
BTF_ID(func, bpf_percpu_obj_new_impl)
1083610843
BTF_ID(func, bpf_percpu_obj_drop_impl)
1083710844
BTF_ID(func, bpf_throw)
10845+
#ifdef CONFIG_CGROUPS
1083810846
BTF_ID(func, bpf_iter_css_task_new)
10847+
#endif
1083910848
BTF_SET_END(special_kfunc_set)
1084010849

1084110850
BTF_ID_LIST(special_kfunc_list)
@@ -10861,7 +10870,11 @@ BTF_ID(func, bpf_dynptr_clone)
1086110870
BTF_ID(func, bpf_percpu_obj_new_impl)
1086210871
BTF_ID(func, bpf_percpu_obj_drop_impl)
1086310872
BTF_ID(func, bpf_throw)
10873+
#ifdef CONFIG_CGROUPS
1086410874
BTF_ID(func, bpf_iter_css_task_new)
10875+
#else
10876+
BTF_ID_UNUSED
10877+
#endif
1086510878

1086610879
static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
1086710880
{
@@ -11394,17 +11407,25 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
1139411407
&meta->arg_rbtree_root.field);
1139511408
}
1139611409

11410+
/*
11411+
* css_task iter allowlist is needed to avoid dead locking on css_set_lock.
11412+
* LSM hooks and iters (both sleepable and non-sleepable) are safe.
11413+
* Any sleepable progs are also safe since bpf_check_attach_target() enforce
11414+
* them can only be attached to some specific hook points.
11415+
*/
1139711416
static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
1139811417
{
1139911418
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
1140011419

1140111420
switch (prog_type) {
1140211421
case BPF_PROG_TYPE_LSM:
1140311422
return true;
11404-
case BPF_TRACE_ITER:
11405-
return env->prog->aux->sleepable;
11423+
case BPF_PROG_TYPE_TRACING:
11424+
if (env->prog->expected_attach_type == BPF_TRACE_ITER)
11425+
return true;
11426+
fallthrough;
1140611427
default:
11407-
return false;
11428+
return env->prog->aux->sleepable;
1140811429
}
1140911430
}
1141011431

@@ -11663,7 +11684,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
1166311684
case KF_ARG_PTR_TO_ITER:
1166411685
if (meta->func_id == special_kfunc_list[KF_bpf_iter_css_task_new]) {
1166511686
if (!check_css_task_iter_allowlist(env)) {
11666-
verbose(env, "css_task_iter is only allowed in bpf_lsm and bpf iter-s\n");
11687+
verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n");
1166711688
return -EINVAL;
1166811689
}
1166911690
}

kernel/cgroup/rstat.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,19 +156,16 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
156156
* optimize away the callsite. Therefore, __weak is needed to ensure that the
157157
* call is still emitted, by telling the compiler that we don't know what the
158158
* function might eventually be.
159-
*
160-
* __diag_* below are needed to dismiss the missing prototype warning.
161159
*/
162-
__diag_push();
163-
__diag_ignore_all("-Wmissing-prototypes",
164-
"kfuncs which will be used in BPF programs");
160+
161+
__bpf_hook_start();
165162

166163
__weak noinline void bpf_rstat_flush(struct cgroup *cgrp,
167164
struct cgroup *parent, int cpu)
168165
{
169166
}
170167

171-
__diag_pop();
168+
__bpf_hook_end();
172169

173170
/* see cgroup_rstat_flush() */
174171
static void cgroup_rstat_flush_locked(struct cgroup *cgrp)

0 commit comments

Comments
 (0)