Merge tag 'bpf-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf

Pull bpf fixes from Alexei Starovoitov: - Replace bpf_map_kmalloc_node() with kmalloc_nolock() to fix kmemleak imbalance in tracking of bpf_async_cb structures (Alexei Starovoitov) - Make selftests/bpf arg_parsing.c more robust to errors (Andrii Nakryiko) - Fix redefinition of 'off' as different kind of symbol when I40E driver is builtin (Brahmajit Das) - Do not disable preemption in bpf_test_run (Sahil Chandna) - Fix memory leak in __lookup_instance error path (Shardul Bankar) - Ensure test data is flushed to disk before reading it (Xing Guo) * tag 'bpf-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: selftests/bpf: Fix redefinition of 'off' as different kind of symbol bpf: Do not disable preemption in bpf_test_run(). bpf: Fix memory leak in __lookup_instance error path selftests: arg_parsing: Ensure data is flushed to disk before reading. bpf: Replace bpf_map_kmalloc_node() with kmalloc_nolock() to allocate bpf_async_cb structures. selftests/bpf: make arg_parsing.c more robust to crashes bpf: test_run: Fix ctx leak in bpf_prog_test_run_xdp error path

Linus Torvalds committed 1 year ago

Commit d303caf2 parents 847f242 a1e83d47 files · +59 −40Browse files

Unified · Split

7 files

include/linux/bpf.h +4 −0
kernel/bpf/helpers.c +14 −11
kernel/bpf/liveness.c +3 −1
kernel/bpf/syscall.c +15 −0
net/bpf/test_run.c +7 −18
tools/testing/selftests/bpf/prog_tests/arg_parsing.c +9 −3
tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c +7 −7

include/linux/bpf.h+4−0

@@ -2499,6 +2499,8 @@ int bpf_map_alloc_pages(const struct bpf_map *map, int nid, · 2498 unmodified lines
2499	2499	#ifdef CONFIG_MEMCG
2500	2500	void bpf_map_kmalloc_node(const struct bpf_map map, size_t size, gfp_t flags,
2501	2501	int node);
	2502	+void bpf_map_kmalloc_nolock(const struct bpf_map map, size_t size, gfp_t flags,
	2503	+ int node);
2502	2504	void bpf_map_kzalloc(const struct bpf_map map, size_t size, gfp_t flags);
2503	2505	void bpf_map_kvcalloc(struct bpf_map map, size_t n, size_t size,
2504	2506	gfp_t flags);
@@ -2511,6 +2513,8 @@ void __percpu bpf_map_alloc_percpu(const struct bpf_map map, size_t size, · 6 unmodified lines
2511	2513	*/
2512	2514	#define bpf_map_kmalloc_node(_map, _size, _flags, _node) \
2513	2515	kmalloc_node(_size, _flags, _node)
	2516	+#define bpf_map_kmalloc_nolock(_map, _size, _flags, _node) \
	2517	+ kmalloc_nolock(_size, _flags, _node)
2514	2518	#define bpf_map_kzalloc(_map, _size, _flags) \
2515	2519	kzalloc(_size, _flags)
2516	2520	#define bpf_map_kvcalloc(_map, _n, _size, _flags) \

kernel/bpf/helpers.c+14−11

@@ -1215,13 +1215,20 @@ static void bpf_wq_work(struct work_struct *work) · 1214 unmodified lines
1215	1215	rcu_read_unlock_trace();
1216	1216	}
1217	1217
	1218	+static void bpf_async_cb_rcu_free(struct rcu_head *rcu)
	1219	+{
	1220	+ struct bpf_async_cb *cb = container_of(rcu, struct bpf_async_cb, rcu);
	1221	+
	1222	+ kfree_nolock(cb);
	1223	+}
	1224	+
1218	1225	static void bpf_wq_delete_work(struct work_struct *work)
1219	1226	{
1220	1227	struct bpf_work *w = container_of(work, struct bpf_work, delete_work);
1221	1228
1222	1229	cancel_work_sync(&w->work);
1223	1230
1224		− kfree_rcu(w, cb.rcu);
	1231	+ call_rcu(&w->cb.rcu, bpf_async_cb_rcu_free);
1225	1232	}
1226	1233
1227	1234	static void bpf_timer_delete_work(struct work_struct *work)
@@ -1230,13 +1237,13 @@ static void bpf_timer_delete_work(struct work_struct *work) · 2 unmodified lines
1230	1237
1231	1238	/* Cancel the timer and wait for callback to complete if it was running.
1232	1239	* If hrtimer_cancel() can be safely called it's safe to call
1233		− * kfree_rcu(t) right after for both preallocated and non-preallocated
	1240	+ * call_rcu() right after for both preallocated and non-preallocated
1234	1241	* maps. The async->cb = NULL was already done and no code path can see
1235	1242	* address 't' anymore. Timer if armed for existing bpf_hrtimer before
1236	1243	* bpf_timer_cancel_and_free will have been cancelled.
1237	1244	*/
1238	1245	hrtimer_cancel(&t->timer);
1239		− kfree_rcu(t, cb.rcu);
	1246	+ call_rcu(&t->cb.rcu, bpf_async_cb_rcu_free);
1240	1247	}
1241	1248
1242	1249	static int __bpf_async_init(struct bpf_async_kern async, struct bpf_map map, u64 flags,
@@ -1270,11 +1277,7 @@ static int __bpf_async_init(struct bpf_async_kern async, struct bpf_map map, u · 27 unmodified lines
1270	1277	goto out;
1271	1278	}
1272	1279
1273		− /* Allocate via bpf_map_kmalloc_node() for memcg accounting. Until
1274		− * kmalloc_nolock() is available, avoid locking issues by using
1275		− * __GFP_HIGH (GFP_ATOMIC & ~__GFP_RECLAIM).
1276		− */
1277		− cb = bpf_map_kmalloc_node(map, size, __GFP_HIGH, map->numa_node);
	1280	+ cb = bpf_map_kmalloc_nolock(map, size, 0, map->numa_node);
1278	1281	if (!cb) {
1279	1282	ret = -ENOMEM;
1280	1283	goto out;
@@ -1315,7 +1318,7 @@ static int __bpf_async_init(struct bpf_async_kern async, struct bpf_map map, u · 34 unmodified lines
1315	1318	* or pinned in bpffs.
1316	1319	*/
1317	1320	WRITE_ONCE(async->cb, NULL);
1318		− kfree(cb);
	1321	+ kfree_nolock(cb);
1319	1322	ret = -EPERM;
1320	1323	}
1321	1324	out:
@@ -1580,7 +1583,7 @@ void bpf_timer_cancel_and_free(void *val) · 258 unmodified lines
1580	1583	* timer _before_ calling us, such that failing to cancel it here will
1581	1584	* cause it to possibly use struct hrtimer after freeing bpf_hrtimer.
1582	1585	* Therefore, we _need_ to cancel any outstanding timers before we do
1583		− * kfree_rcu, even though no more timers can be armed.
	1586	+ * call_rcu, even though no more timers can be armed.
1584	1587	*
1585	1588	* Moreover, we need to schedule work even if timer does not belong to
1586	1589	* the calling callback_fn, as on two different CPUs, we can end up in a
@@ -1607,7 +1610,7 @@ void bpf_timer_cancel_and_free(void *val) · 20 unmodified lines
1607	1610	* completion.
1608	1611	*/
1609	1612	if (hrtimer_try_to_cancel(&t->timer) >= 0)
1610		− kfree_rcu(t, cb.rcu);
	1613	+ call_rcu(&t->cb.rcu, bpf_async_cb_rcu_free);
1611	1614	else
1612	1615	queue_work(system_dfl_wq, &t->cb.delete_work);
1613	1616	} else {

kernel/bpf/liveness.c+3−1

@@ -195,8 +195,10 @@ static struct func_instance __lookup_instance(struct bpf_verifier_env env, · 194 unmodified lines
195	195	return ERR_PTR(-ENOMEM);
196	196	result->must_write_set = kvcalloc(subprog_sz, sizeof(*result->must_write_set),
197	197	GFP_KERNEL_ACCOUNT);
198		− if (!result->must_write_set)
	198	+ if (!result->must_write_set) {
	199	+ kvfree(result);
199	200	return ERR_PTR(-ENOMEM);
	201	+ }
200	202	memcpy(&result->callchain, callchain, sizeof(*callchain));
201	203	result->insn_cnt = subprog_sz;
202	204	hash_add(liveness->func_instances, &result->hl_node, key);

kernel/bpf/syscall.c+15−0

@@ -520,6 +520,21 @@ void bpf_map_kmalloc_node(const struct bpf_map map, size_t size, gfp_t flags, · 519 unmodified lines
520	520	return ptr;
521	521	}
522	522
	523	+void bpf_map_kmalloc_nolock(const struct bpf_map map, size_t size, gfp_t flags,
	524	+ int node)
	525	+{
	526	+ struct mem_cgroup memcg, old_memcg;
	527	+ void *ptr;
	528	+
	529	+ memcg = bpf_map_get_memcg(map);
	530	+ old_memcg = set_active_memcg(memcg);
	531	+ ptr = kmalloc_nolock(size, flags \| __GFP_ACCOUNT, node);
	532	+ set_active_memcg(old_memcg);
	533	+ mem_cgroup_put(memcg);
	534	+
	535	+ return ptr;
	536	+}
	537	+
523	538	void bpf_map_kzalloc(const struct bpf_map map, size_t size, gfp_t flags)
524	539	{
525	540	struct mem_cgroup memcg, old_memcg;

net/bpf/test_run.c+7−18

@@ -29,7 +29,6 @@ · 28 unmodified lines
29	29	#include <trace/events/bpf_test_run.h>
30	30
31	31	struct bpf_test_timer {
32		− enum { NO_PREEMPT, NO_MIGRATE } mode;
33	32	u32 i;
34	33	u64 time_start, time_spent;
35	34	};
@@ -37,12 +36,7 @@ struct bpf_test_timer { · 1 unmodified line
37	36	static void bpf_test_timer_enter(struct bpf_test_timer *t)
38	37	__acquires(rcu)
39	38	{
40		− rcu_read_lock();
41		− if (t->mode == NO_PREEMPT)
42		− preempt_disable();
43		− else
44		− migrate_disable();
45		−
	39	+ rcu_read_lock_dont_migrate();
46	40	t->time_start = ktime_get_ns();
47	41	}
48	42
@@ -50,12 +44,7 @@ static void bpf_test_timer_leave(struct bpf_test_timer *t) · 1 unmodified line
50	44	__releases(rcu)
51	45	{
52	46	t->time_start = 0;
53		−
54		− if (t->mode == NO_PREEMPT)
55		− preempt_enable();
56		− else
57		− migrate_enable();
58		− rcu_read_unlock();
	47	+ rcu_read_unlock_migrate();
59	48	}
60	49
61	50	static bool bpf_test_timer_continue(struct bpf_test_timer *t, int iterations,
@@ -374,7 +363,7 @@ static int bpf_test_run_xdp_live(struct bpf_prog prog, struct xdp_buff ctx, · 312 unmodified lines
374	363
375	364	{
376	365	struct xdp_test_data xdp = { .batch_size = batch_size };
377		− struct bpf_test_timer t = { .mode = NO_MIGRATE };
	366	+ struct bpf_test_timer t = {};
378	367	int ret;
379	368
380	369	if (!repeat)
@@ -404,7 +393,7 @@ static int bpf_test_run(struct bpf_prog prog, void ctx, u32 repeat, · 23 unmodified lines
404	393	struct bpf_prog_array_item item = {.prog = prog};
405	394	struct bpf_run_ctx *old_ctx;
406	395	struct bpf_cg_run_ctx run_ctx;
407		− struct bpf_test_timer t = { NO_MIGRATE };
	396	+ struct bpf_test_timer t = {};
408	397	enum bpf_cgroup_storage_type stype;
409	398	int ret;
410	399
@@ -1269,7 +1258,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog prog, const union bpf_attr kattr, · 858 unmodified lines
1269	1258	goto free_ctx;
1270	1259
1271	1260	if (kattr->test.data_size_in - meta_sz < ETH_HLEN)
1272		− return -EINVAL;
	1261	+ goto free_ctx;
1273	1262
1274	1263	data = bpf_test_init(kattr, linear_sz, max_linear_sz, headroom, tailroom);
1275	1264	if (IS_ERR(data)) {
@@ -1377,7 +1366,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, · 101 unmodified lines
1377	1366	const union bpf_attr *kattr,
1378	1367	union bpf_attr __user *uattr)
1379	1368	{
1380		− struct bpf_test_timer t = { NO_PREEMPT };
	1369	+ struct bpf_test_timer t = {};
1381	1370	u32 size = kattr->test.data_size_in;
1382	1371	struct bpf_flow_dissector ctx = {};
1383	1372	u32 repeat = kattr->test.repeat;
@@ -1445,7 +1434,7 @@ out: · 61 unmodified lines
1445	1434	int bpf_prog_test_run_sk_lookup(struct bpf_prog prog, const union bpf_attr kattr,
1446	1435	union bpf_attr __user *uattr)
1447	1436	{
1448		− struct bpf_test_timer t = { NO_PREEMPT };
	1437	+ struct bpf_test_timer t = {};
1449	1438	struct bpf_prog_array *progs = NULL;
1450	1439	struct bpf_sk_lookup_kern ctx = {};
1451	1440	u32 repeat = kattr->test.repeat;

tools/testing/selftests/bpf/prog_tests/arg_parsing.c+9−3

@@ -144,11 +144,17 @@ static void test_parse_test_list_file(void) · 143 unmodified lines
144	144	if (!ASSERT_OK(ferror(fp), "prepare tmp"))
145	145	goto out_fclose;
146	146
	147	+ if (!ASSERT_OK(fsync(fileno(fp)), "fsync tmp"))
	148	+ goto out_fclose;
	149	+
147	150	init_test_filter_set(&set);
148	151
149		− ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file");
	152	+ if (!ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file"))
	153	+ goto out_fclose;
	154	+
	155	+ if (!ASSERT_EQ(set.cnt, 4, "test count"))
	156	+ goto out_free_set;
150	157
151		− ASSERT_EQ(set.cnt, 4, "test count");
152	158	ASSERT_OK(strcmp("test_with_spaces", set.tests[0].name), "test 0 name");
153	159	ASSERT_EQ(set.tests[0].subtest_cnt, 0, "test 0 subtest count");
154	160	ASSERT_OK(strcmp("testA", set.tests[1].name), "test 1 name");
@@ -158,8 +164,8 @@ static void test_parse_test_list_file(void) · 3 unmodified lines
158	164	ASSERT_OK(strcmp("testB", set.tests[2].name), "test 2 name");
159	165	ASSERT_OK(strcmp("testC_no_eof_newline", set.tests[3].name), "test 3 name");
160	166
	167	+out_free_set:
161	168	free_test_filter_set(&set);
162		−
163	169	out_fclose:
164	170	fclose(fp);
165	171	out_remove:

tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c+7−7

@@ -225,7 +225,7 @@ int trusted_to_untrusted(void *ctx) · 224 unmodified lines
225	225	}
226	226
227	227	char mem[16];
228		−u32 off;
	228	+u32 offset;
229	229
230	230	SEC("tp_btf/sys_enter")
231	231	__success
@@ -240,9 +240,9 @@ int anything_to_untrusted(void *ctx) · 8 unmodified lines
240	240	/* scalar to untrusted */
241	241	subprog_untrusted(0);
242	242	/* variable offset to untrusted (map) */
243		− subprog_untrusted((void *)mem + off);
	243	+ subprog_untrusted((void *)mem + offset);
244	244	/* variable offset to untrusted (trusted) */
245		− subprog_untrusted((void *)bpf_get_current_task_btf() + off);
	245	+ subprog_untrusted((void *)bpf_get_current_task_btf() + offset);
246	246	return 0;
247	247	}
248	248
@@ -298,12 +298,12 @@ int anything_to_untrusted_mem(void *ctx) · 49 unmodified lines
298	298	/* scalar to untrusted mem */
299	299	subprog_void_untrusted(0);
300	300	/* variable offset to untrusted mem (map) */
301		− subprog_void_untrusted((void *)mem + off);
	301	+ subprog_void_untrusted((void *)mem + offset);
302	302	/* variable offset to untrusted mem (trusted) */
303		− subprog_void_untrusted(bpf_get_current_task_btf() + off);
	303	+ subprog_void_untrusted(bpf_get_current_task_btf() + offset);
304	304	/* variable offset to untrusted char/enum (map) */
305		− subprog_char_untrusted(mem + off);
306		− subprog_enum_untrusted((void *)mem + off);
	305	+ subprog_char_untrusted(mem + offset);
	306	+ subprog_enum_untrusted((void *)mem + offset);
307	307	return 0;
308	308	}
309	309