Skip to content

Commit 59579ef

Browse files
jsitnickiKernel Patches Daemon
authored andcommitted
bpf, verifier: Remove side effects from may_access_direct_pkt_data
The may_access_direct_pkt_data() helper sets env->seen_direct_write as a side effect, which creates awkward calling patterns: - check_special_kfunc() has a comment warning readers about the side effect - specialize_kfunc() must save and restore the flag around the call Make the helper a pure function by moving the seen_direct_write flag setting to call sites that need it. Signed-off-by: Jakub Sitnicki <[email protected]>
1 parent 8092a12 commit 59579ef

File tree

1 file changed

+12
-21
lines changed

1 file changed

+12
-21
lines changed

kernel/bpf/verifier.c

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6085,13 +6085,9 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
60856085
if (meta)
60866086
return meta->pkt_access;
60876087

6088-
env->seen_direct_write = true;
60896088
return true;
60906089

60916090
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
6092-
if (t == BPF_WRITE)
6093-
env->seen_direct_write = true;
6094-
60956091
return true;
60966092

60976093
default:
@@ -7619,15 +7615,17 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
76197615
err = check_stack_write(env, regno, off, size,
76207616
value_regno, insn_idx);
76217617
} else if (reg_is_pkt_pointer(reg)) {
7622-
if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL, t)) {
7623-
verbose(env, "cannot write into packet\n");
7624-
return -EACCES;
7625-
}
7626-
if (t == BPF_WRITE && value_regno >= 0 &&
7627-
is_pointer_value(env, value_regno)) {
7628-
verbose(env, "R%d leaks addr into packet\n",
7629-
value_regno);
7630-
return -EACCES;
7618+
if (t == BPF_WRITE) {
7619+
if (!may_access_direct_pkt_data(env, NULL, BPF_WRITE)) {
7620+
verbose(env, "cannot write into packet\n");
7621+
return -EACCES;
7622+
}
7623+
if (value_regno >= 0 && is_pointer_value(env, value_regno)) {
7624+
verbose(env, "R%d leaks addr into packet\n",
7625+
value_regno);
7626+
return -EACCES;
7627+
}
7628+
env->seen_direct_write = true;
76317629
}
76327630
err = check_packet_access(env, regno, off, size, false);
76337631
if (!err && t == BPF_READ && value_regno >= 0)
@@ -13766,11 +13764,11 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
1376613764
if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice]) {
1376713765
regs[BPF_REG_0].type |= MEM_RDONLY;
1376813766
} else {
13769-
/* this will set env->seen_direct_write to true */
1377013767
if (!may_access_direct_pkt_data(env, NULL, BPF_WRITE)) {
1377113768
verbose(env, "the prog does not allow writes to packet data\n");
1377213769
return -EINVAL;
1377313770
}
13771+
env->seen_direct_write = true;
1377413772
}
1377513773

1377613774
if (!meta->initialized_dynptr.id) {
@@ -21810,7 +21808,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
2181021808
u32 func_id, u16 offset, unsigned long *addr)
2181121809
{
2181221810
struct bpf_prog *prog = env->prog;
21813-
bool seen_direct_write;
2181421811
void *xdp_kfunc;
2181521812
bool is_rdonly;
2181621813

@@ -21827,16 +21824,10 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
2182721824
return;
2182821825

2182921826
if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
21830-
seen_direct_write = env->seen_direct_write;
2183121827
is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE);
2183221828

2183321829
if (is_rdonly)
2183421830
*addr = (unsigned long)bpf_dynptr_from_skb_rdonly;
21835-
21836-
/* restore env->seen_direct_write to its original value, since
21837-
* may_access_direct_pkt_data mutates it
21838-
*/
21839-
env->seen_direct_write = seen_direct_write;
2184021831
}
2184121832

2184221833
if (func_id == special_kfunc_list[KF_bpf_set_dentry_xattr] &&

0 commit comments

Comments
 (0)