Skip to content

Commit b0b0ab6

Browse files
committed
Alexei Starovoitov says: ==================== pull-request: bpf 2023-07-12 We've added 5 non-merge commits during the last 7 day(s) which contain a total of 7 files changed, 93 insertions(+), 28 deletions(-). The main changes are: 1) Fix max stack depth check for async callbacks, from Kumar. 2) Fix inconsistent JIT image generation, from Björn. 3) Use trusted arguments in XDP hints kfuncs, from Larysa. 4) Fix memory leak in cpu_map_update_elem, from Pu. * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: xdp: use trusted arguments in XDP hints kfuncs bpf: cpumap: Fix memory leak in cpu_map_update_elem riscv, bpf: Fix inconsistent JIT image generation selftests/bpf: Add selftest for check_stack_max_depth bug bpf: Fix max stack depth check for async callbacks ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents fec3ebb + 2e06c57 commit b0b0ab6

File tree

7 files changed

+93
-28
lines changed

7 files changed

+93
-28
lines changed

arch/riscv/net/bpf_jit.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ struct rv_jit_context {
6969
struct bpf_prog *prog;
7070
u16 *insns; /* RV insns */
7171
int ninsns;
72-
int body_len;
72+
int prologue_len;
7373
int epilogue_offset;
7474
int *offset; /* BPF to RV */
7575
int nexentries;
@@ -216,8 +216,8 @@ static inline int rv_offset(int insn, int off, struct rv_jit_context *ctx)
216216
int from, to;
217217

218218
off++; /* BPF branch is from PC+1, RV is from PC */
219-
from = (insn > 0) ? ctx->offset[insn - 1] : 0;
220-
to = (insn + off > 0) ? ctx->offset[insn + off - 1] : 0;
219+
from = (insn > 0) ? ctx->offset[insn - 1] : ctx->prologue_len;
220+
to = (insn + off > 0) ? ctx->offset[insn + off - 1] : ctx->prologue_len;
221221
return ninsns_rvoff(to - from);
222222
}
223223

arch/riscv/net/bpf_jit_core.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
4444
unsigned int prog_size = 0, extable_size = 0;
4545
bool tmp_blinded = false, extra_pass = false;
4646
struct bpf_prog *tmp, *orig_prog = prog;
47-
int pass = 0, prev_ninsns = 0, prologue_len, i;
47+
int pass = 0, prev_ninsns = 0, i;
4848
struct rv_jit_data *jit_data;
4949
struct rv_jit_context *ctx;
5050

@@ -83,6 +83,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
8383
prog = orig_prog;
8484
goto out_offset;
8585
}
86+
87+
if (build_body(ctx, extra_pass, NULL)) {
88+
prog = orig_prog;
89+
goto out_offset;
90+
}
91+
8692
for (i = 0; i < prog->len; i++) {
8793
prev_ninsns += 32;
8894
ctx->offset[i] = prev_ninsns;
@@ -91,12 +97,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
9197
for (i = 0; i < NR_JIT_ITERATIONS; i++) {
9298
pass++;
9399
ctx->ninsns = 0;
100+
101+
bpf_jit_build_prologue(ctx);
102+
ctx->prologue_len = ctx->ninsns;
103+
94104
if (build_body(ctx, extra_pass, ctx->offset)) {
95105
prog = orig_prog;
96106
goto out_offset;
97107
}
98-
ctx->body_len = ctx->ninsns;
99-
bpf_jit_build_prologue(ctx);
108+
100109
ctx->epilogue_offset = ctx->ninsns;
101110
bpf_jit_build_epilogue(ctx);
102111

@@ -162,10 +171,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
162171

163172
if (!prog->is_func || extra_pass) {
164173
bpf_jit_binary_lock_ro(jit_data->header);
165-
prologue_len = ctx->epilogue_offset - ctx->body_len;
166174
for (i = 0; i < prog->len; i++)
167-
ctx->offset[i] = ninsns_rvoff(prologue_len +
168-
ctx->offset[i]);
175+
ctx->offset[i] = ninsns_rvoff(ctx->offset[i]);
169176
bpf_prog_fill_jited_linfo(prog, ctx->offset);
170177
out_offset:
171178
kfree(ctx->offset);

kernel/bpf/cpumap.c

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -122,22 +122,6 @@ static void get_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
122122
atomic_inc(&rcpu->refcnt);
123123
}
124124

125-
/* called from workqueue, to workaround syscall using preempt_disable */
126-
static void cpu_map_kthread_stop(struct work_struct *work)
127-
{
128-
struct bpf_cpu_map_entry *rcpu;
129-
130-
rcpu = container_of(work, struct bpf_cpu_map_entry, kthread_stop_wq);
131-
132-
/* Wait for flush in __cpu_map_entry_free(), via full RCU barrier,
133-
* as it waits until all in-flight call_rcu() callbacks complete.
134-
*/
135-
rcu_barrier();
136-
137-
/* kthread_stop will wake_up_process and wait for it to complete */
138-
kthread_stop(rcpu->kthread);
139-
}
140-
141125
static void __cpu_map_ring_cleanup(struct ptr_ring *ring)
142126
{
143127
/* The tear-down procedure should have made sure that queue is
@@ -165,6 +149,30 @@ static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
165149
}
166150
}
167151

152+
/* called from workqueue, to workaround syscall using preempt_disable */
153+
static void cpu_map_kthread_stop(struct work_struct *work)
154+
{
155+
struct bpf_cpu_map_entry *rcpu;
156+
int err;
157+
158+
rcpu = container_of(work, struct bpf_cpu_map_entry, kthread_stop_wq);
159+
160+
/* Wait for flush in __cpu_map_entry_free(), via full RCU barrier,
161+
* as it waits until all in-flight call_rcu() callbacks complete.
162+
*/
163+
rcu_barrier();
164+
165+
/* kthread_stop will wake_up_process and wait for it to complete */
166+
err = kthread_stop(rcpu->kthread);
167+
if (err) {
168+
/* kthread_stop may be called before cpu_map_kthread_run
169+
* is executed, so we need to release the memory related
170+
* to rcpu.
171+
*/
172+
put_cpu_map_entry(rcpu);
173+
}
174+
}
175+
168176
static void cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu,
169177
struct list_head *listp,
170178
struct xdp_cpumap_stats *stats)

kernel/bpf/verifier.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5642,8 +5642,9 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
56425642
verbose(env, "verifier bug. subprog has tail_call and async cb\n");
56435643
return -EFAULT;
56445644
}
5645-
/* async callbacks don't increase bpf prog stack size */
5646-
continue;
5645+
/* async callbacks don't increase bpf prog stack size unless called directly */
5646+
if (!bpf_pseudo_call(insn + i))
5647+
continue;
56475648
}
56485649
i = next_insn;
56495650

net/core/xdp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
741741
__diag_pop();
742742

743743
BTF_SET8_START(xdp_metadata_kfunc_ids)
744-
#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, 0)
744+
#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
745745
XDP_METADATA_KFUNC_xxx
746746
#undef XDP_METADATA_KFUNC
747747
BTF_SET8_END(xdp_metadata_kfunc_ids)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
#include <test_progs.h>
3+
4+
#include "async_stack_depth.skel.h"
5+
6+
void test_async_stack_depth(void)
7+
{
8+
RUN_TESTS(async_stack_depth);
9+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
#include <vmlinux.h>
3+
#include <bpf/bpf_helpers.h>
4+
5+
#include "bpf_misc.h"
6+
7+
struct hmap_elem {
8+
struct bpf_timer timer;
9+
};
10+
11+
struct {
12+
__uint(type, BPF_MAP_TYPE_HASH);
13+
__uint(max_entries, 64);
14+
__type(key, int);
15+
__type(value, struct hmap_elem);
16+
} hmap SEC(".maps");
17+
18+
__attribute__((noinline))
19+
static int timer_cb(void *map, int *key, struct bpf_timer *timer)
20+
{
21+
volatile char buf[256] = {};
22+
return buf[69];
23+
}
24+
25+
SEC("tc")
26+
__failure __msg("combined stack size of 2 calls")
27+
int prog(struct __sk_buff *ctx)
28+
{
29+
struct hmap_elem *elem;
30+
volatile char buf[256] = {};
31+
32+
elem = bpf_map_lookup_elem(&hmap, &(int){0});
33+
if (!elem)
34+
return 0;
35+
36+
timer_cb(NULL, NULL, NULL);
37+
return bpf_timer_set_callback(&elem->timer, timer_cb) + buf[0];
38+
}
39+
40+
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)