Skip to content

Commit 105faa8

Browse files
kuba-mookuba-moo
committed
Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
Daniel Borkmann says: ==================== pull-request: bpf-next 2020-10-15 The main changes are: 1) Fix register equivalence tracking in verifier, from Alexei Starovoitov. 2) Fix sockmap error path to not call bpf_prog_put() with NULL, from Alex Dewar. 3) Fix sockmap to add locking annotations to iterator, from Lorenz Bauer. 4) Fix tcp_hdr_options test to use loopback address, from Martin KaFai Lau. ==================== Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 2295cdd + 83c11c1 commit 105faa8

File tree

5 files changed

+70
-46
lines changed

5 files changed

+70
-46
lines changed

‎kernel/bpf/verifier.c

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,14 +1010,9 @@ static const int caller_saved[CALLER_SAVED_REGS] = {
10101010
static void __mark_reg_not_init(const struct bpf_verifier_env *env,
10111011
struct bpf_reg_state *reg);
10121012

1013-
/* Mark the unknown part of a register (variable offset or scalar value) as
1014-
* known to have the value @imm.
1015-
*/
1016-
static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
1013+
/* This helper doesn't clear reg->id */
1014+
static void ___mark_reg_known(struct bpf_reg_state *reg, u64 imm)
10171015
{
1018-
/* Clear id, off, and union(map_ptr, range) */
1019-
memset(((u8 *)reg) + sizeof(reg->type), 0,
1020-
offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
10211016
reg->var_off = tnum_const(imm);
10221017
reg->smin_value = (s64)imm;
10231018
reg->smax_value = (s64)imm;
@@ -1030,6 +1025,17 @@ static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
10301025
reg->u32_max_value = (u32)imm;
10311026
}
10321027

1028+
/* Mark the unknown part of a register (variable offset or scalar value) as
1029+
* known to have the value @imm.
1030+
*/
1031+
static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
1032+
{
1033+
/* Clear id, off, and union(map_ptr, range) */
1034+
memset(((u8 *)reg) + sizeof(reg->type), 0,
1035+
offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
1036+
___mark_reg_known(reg, imm);
1037+
}
1038+
10331039
static void __mark_reg32_known(struct bpf_reg_state *reg, u64 imm)
10341040
{
10351041
reg->var_off = tnum_const_subreg(reg->var_off, imm);
@@ -7001,14 +7007,18 @@ static void reg_set_min_max(struct bpf_reg_state *true_reg,
70017007
struct bpf_reg_state *reg =
70027008
opcode == BPF_JEQ ? true_reg : false_reg;
70037009

7004-
/* For BPF_JEQ, if this is false we know nothing Jon Snow, but
7005-
* if it is true we know the value for sure. Likewise for
7006-
* BPF_JNE.
7010+
/* JEQ/JNE comparison doesn't change the register equivalence.
7011+
* r1 = r2;
7012+
* if (r1 == 42) goto label;
7013+
* ...
7014+
* label: // here both r1 and r2 are known to be 42.
7015+
*
7016+
* Hence when marking register as known preserve it's ID.
70077017
*/
70087018
if (is_jmp32)
70097019
__mark_reg32_known(reg, val32);
70107020
else
7011-
__mark_reg_known(reg, val);
7021+
___mark_reg_known(reg, val);
70127022
break;
70137023
}
70147024
case BPF_JSET:
@@ -7551,7 +7561,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
75517561
reg_combine_min_max(&other_branch_regs[insn->src_reg],
75527562
&other_branch_regs[insn->dst_reg],
75537563
src_reg, dst_reg, opcode);
7554-
if (src_reg->id) {
7564+
if (src_reg->id &&
7565+
!WARN_ON_ONCE(src_reg->id != other_branch_regs[insn->src_reg].id)) {
75557566
find_equal_scalars(this_branch, src_reg);
75567567
find_equal_scalars(other_branch, &other_branch_regs[insn->src_reg]);
75577568
}
@@ -7563,7 +7574,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
75637574
opcode, is_jmp32);
75647575
}
75657576

7566-
if (dst_reg->type == SCALAR_VALUE && dst_reg->id) {
7577+
if (dst_reg->type == SCALAR_VALUE && dst_reg->id &&
7578+
!WARN_ON_ONCE(dst_reg->id != other_branch_regs[insn->dst_reg].id)) {
75677579
find_equal_scalars(this_branch, dst_reg);
75687580
find_equal_scalars(other_branch, &other_branch_regs[insn->dst_reg]);
75697581
}

‎net/core/sock_map.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,17 +238,18 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs,
238238
int ret;
239239

240240
skb_verdict = READ_ONCE(progs->skb_verdict);
241-
skb_parser = READ_ONCE(progs->skb_parser);
242241
if (skb_verdict) {
243242
skb_verdict = bpf_prog_inc_not_zero(skb_verdict);
244243
if (IS_ERR(skb_verdict))
245244
return PTR_ERR(skb_verdict);
246245
}
246+
247+
skb_parser = READ_ONCE(progs->skb_parser);
247248
if (skb_parser) {
248249
skb_parser = bpf_prog_inc_not_zero(skb_parser);
249250
if (IS_ERR(skb_parser)) {
250-
bpf_prog_put(skb_verdict);
251-
return PTR_ERR(skb_parser);
251+
ret = PTR_ERR(skb_parser);
252+
goto out_put_skb_verdict;
252253
}
253254
}
254255

@@ -257,7 +258,7 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs,
257258
msg_parser = bpf_prog_inc_not_zero(msg_parser);
258259
if (IS_ERR(msg_parser)) {
259260
ret = PTR_ERR(msg_parser);
260-
goto out;
261+
goto out_put_skb_parser;
261262
}
262263
}
263264

@@ -311,11 +312,12 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs,
311312
out_progs:
312313
if (msg_parser)
313314
bpf_prog_put(msg_parser);
314-
out:
315-
if (skb_verdict)
316-
bpf_prog_put(skb_verdict);
315+
out_put_skb_parser:
317316
if (skb_parser)
318317
bpf_prog_put(skb_parser);
318+
out_put_skb_verdict:
319+
if (skb_verdict)
320+
bpf_prog_put(skb_verdict);
319321
return ret;
320322
}
321323

@@ -745,6 +747,7 @@ static void *sock_map_seq_lookup_elem(struct sock_map_seq_info *info)
745747
}
746748

747749
static void *sock_map_seq_start(struct seq_file *seq, loff_t *pos)
750+
__acquires(rcu)
748751
{
749752
struct sock_map_seq_info *info = seq->private;
750753

@@ -757,6 +760,7 @@ static void *sock_map_seq_start(struct seq_file *seq, loff_t *pos)
757760
}
758761

759762
static void *sock_map_seq_next(struct seq_file *seq, void *v, loff_t *pos)
763+
__must_hold(rcu)
760764
{
761765
struct sock_map_seq_info *info = seq->private;
762766

@@ -767,6 +771,7 @@ static void *sock_map_seq_next(struct seq_file *seq, void *v, loff_t *pos)
767771
}
768772

769773
static int sock_map_seq_show(struct seq_file *seq, void *v)
774+
__must_hold(rcu)
770775
{
771776
struct sock_map_seq_info *info = seq->private;
772777
struct bpf_iter__sockmap ctx = {};
@@ -789,6 +794,7 @@ static int sock_map_seq_show(struct seq_file *seq, void *v)
789794
}
790795

791796
static void sock_map_seq_stop(struct seq_file *seq, void *v)
797+
__releases(rcu)
792798
{
793799
if (!v)
794800
(void)sock_map_seq_show(seq, NULL);
@@ -1353,6 +1359,7 @@ static void *sock_hash_seq_find_next(struct sock_hash_seq_info *info,
13531359
}
13541360

13551361
static void *sock_hash_seq_start(struct seq_file *seq, loff_t *pos)
1362+
__acquires(rcu)
13561363
{
13571364
struct sock_hash_seq_info *info = seq->private;
13581365

@@ -1365,6 +1372,7 @@ static void *sock_hash_seq_start(struct seq_file *seq, loff_t *pos)
13651372
}
13661373

13671374
static void *sock_hash_seq_next(struct seq_file *seq, void *v, loff_t *pos)
1375+
__must_hold(rcu)
13681376
{
13691377
struct sock_hash_seq_info *info = seq->private;
13701378

@@ -1373,6 +1381,7 @@ static void *sock_hash_seq_next(struct seq_file *seq, void *v, loff_t *pos)
13731381
}
13741382

13751383
static int sock_hash_seq_show(struct seq_file *seq, void *v)
1384+
__must_hold(rcu)
13761385
{
13771386
struct sock_hash_seq_info *info = seq->private;
13781387
struct bpf_iter__sockmap ctx = {};
@@ -1396,6 +1405,7 @@ static int sock_hash_seq_show(struct seq_file *seq, void *v)
13961405
}
13971406

13981407
static void sock_hash_seq_stop(struct seq_file *seq, void *v)
1408+
__releases(rcu)
13991409
{
14001410
if (!v)
14011411
(void)sock_hash_seq_show(seq, NULL);

‎tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
#include "test_tcp_hdr_options.skel.h"
1616
#include "test_misc_tcp_hdr_options.skel.h"
1717

18-
#define LO_ADDR6 "::eB9F"
18+
#define LO_ADDR6 "::1"
1919
#define CG_NAME "/tcpbpf-hdr-opt-test"
2020

2121
struct bpf_test_option exp_passive_estab_in;
@@ -40,27 +40,6 @@ struct sk_fds {
4040
int active_lport;
4141
};
4242

43-
static int add_lo_addr(void)
44-
{
45-
char ip_addr_cmd[256];
46-
int cmdlen;
47-
48-
cmdlen = snprintf(ip_addr_cmd, sizeof(ip_addr_cmd),
49-
"ip -6 addr add %s/128 dev lo scope host",
50-
LO_ADDR6);
51-
52-
if (CHECK(cmdlen >= sizeof(ip_addr_cmd), "compile ip cmd",
53-
"failed to add host addr %s to lo. ip cmdlen is too long\n",
54-
LO_ADDR6))
55-
return -1;
56-
57-
if (CHECK(system(ip_addr_cmd), "run ip cmd",
58-
"failed to add host addr %s to lo\n", LO_ADDR6))
59-
return -1;
60-
61-
return 0;
62-
}
63-
6443
static int create_netns(void)
6544
{
6645
if (CHECK(unshare(CLONE_NEWNET), "create netns",
@@ -72,9 +51,6 @@ static int create_netns(void)
7251
"failed to bring lo link up\n"))
7352
return -1;
7453

75-
if (add_lo_addr())
76-
return -1;
77-
7854
return 0;
7955
}
8056

‎tools/testing/selftests/bpf/progs/test_misc_tcp_hdr_options.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#define BPF_PROG_TEST_TCP_HDR_OPTIONS
1717
#include "test_tcp_hdr_options.h"
1818

19-
__u16 last_addr16_n = __bpf_htons(0xeB9F);
19+
__u16 last_addr16_n = __bpf_htons(1);
2020
__u16 active_lport_n = 0;
2121
__u16 active_lport_h = 0;
2222
__u16 passive_lport_n = 0;

‎tools/testing/selftests/bpf/verifier/regalloc.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,29 @@
241241
.result = ACCEPT,
242242
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
243243
},
244+
{
245+
"regalloc, spill, JEQ",
246+
.insns = {
247+
BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
248+
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
249+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
250+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
251+
BPF_LD_MAP_FD(BPF_REG_1, 0),
252+
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
253+
BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_0, -8), /* spill r0 */
254+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 0),
255+
/* The verifier will walk the rest twice with r0 == 0 and r0 == map_value */
256+
BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32),
257+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
258+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 20, 0),
259+
/* The verifier will walk the rest two more times with r0 == 20 and r0 == unknown */
260+
BPF_LDX_MEM(BPF_DW, BPF_REG_3, BPF_REG_10, -8), /* fill r3 with map_value */
261+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_3, 0, 1), /* skip ldx if map_value == NULL */
262+
/* Buggy verifier will think that r3 == 20 here */
263+
BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_3, 0), /* read from map_value */
264+
BPF_EXIT_INSN(),
265+
},
266+
.fixup_map_hash_48b = { 4 },
267+
.result = ACCEPT,
268+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
269+
},

0 commit comments

Comments
 (0)