Skip to content

Commit 3e272a8

Browse files
borkmanngregkh
borkmann
authored and
gregkh
committed
bpf: allow xadd only on aligned memory
[ upstream commit ca36960 ] The requirements around atomic_add() / atomic64_add() resp. their JIT implementations differ across architectures. E.g. while x86_64 seems just fine with BPF's xadd on unaligned memory, on arm64 it triggers via interpreter but also JIT the following crash: [ 830.864985] Unable to handle kernel paging request at virtual address ffff8097d7ed6703 [...] [ 830.916161] Internal error: Oops: 96000021 [#1] SMP [ 830.984755] CPU: 37 PID: 2788 Comm: test_verifier Not tainted 4.16.0-rc2+ #8 [ 830.991790] Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.29 07/17/2017 [ 830.998998] pstate: 80400005 (Nzcv daif +PAN -UAO) [ 831.003793] pc : __ll_sc_atomic_add+0x4/0x18 [ 831.008055] lr : ___bpf_prog_run+0x1198/0x1588 [ 831.012485] sp : ffff00001ccabc20 [ 831.015786] x29: ffff00001ccabc20 x28: ffff8017d56a0f00 [ 831.021087] x27: 0000000000000001 x26: 0000000000000000 [ 831.026387] x25: 000000c168d9db98 x24: 0000000000000000 [ 831.031686] x23: ffff000008203878 x22: ffff000009488000 [ 831.036986] x21: ffff000008b14e28 x20: ffff00001ccabcb0 [ 831.042286] x19: ffff0000097b5080 x18: 0000000000000a03 [ 831.047585] x17: 0000000000000000 x16: 0000000000000000 [ 831.052885] x15: 0000ffffaeca8000 x14: 0000000000000000 [ 831.058184] x13: 0000000000000000 x12: 0000000000000000 [ 831.063484] x11: 0000000000000001 x10: 0000000000000000 [ 831.068783] x9 : 0000000000000000 x8 : 0000000000000000 [ 831.074083] x7 : 0000000000000000 x6 : 000580d428000000 [ 831.079383] x5 : 0000000000000018 x4 : 0000000000000000 [ 831.084682] x3 : ffff00001ccabcb0 x2 : 0000000000000001 [ 831.089982] x1 : ffff8097d7ed6703 x0 : 0000000000000001 [ 831.095282] Process test_verifier (pid: 2788, stack limit = 0x0000000018370044) [ 831.102577] Call trace: [ 831.105012] __ll_sc_atomic_add+0x4/0x18 [ 831.108923] __bpf_prog_run32+0x4c/0x70 [ 831.112748] bpf_test_run+0x78/0xf8 [ 831.116224] bpf_prog_test_run_xdp+0xb4/0x120 [ 831.120567] SyS_bpf+0x77c/0x1110 [ 831.123873] el0_svc_naked+0x30/0x34 [ 831.127437] Code: 97fffe97 17ffffec 00000000 f9800031 (885f7c31) Reason for this is because memory is required to be aligned. In case of BPF, we always enforce alignment in terms of stack access, but not when accessing map values or packet data when the underlying arch (e.g. arm64) has CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS set. xadd on packet data that is local to us anyway is just wrong, so forbid this case entirely. The only place where xadd makes sense in fact are map values; xadd on stack is wrong as well, but it's been around for much longer. Specifically enforce strict alignment in case of xadd, so that we handle this case generically and avoid such crashes in the first place. Fixes: 17a5267 ("bpf: verifier (add verifier core)") Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent e1760b3 commit 3e272a8

File tree

2 files changed

+84
-16
lines changed

2 files changed

+84
-16
lines changed

‎kernel/bpf/verifier.c

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,13 @@ static bool is_ctx_reg(struct bpf_verifier_env *env, int regno)
993993
return reg->type == PTR_TO_CTX;
994994
}
995995

996+
static bool is_pkt_reg(struct bpf_verifier_env *env, int regno)
997+
{
998+
const struct bpf_reg_state *reg = &env->cur_state.regs[regno];
999+
1000+
return reg->type == PTR_TO_PACKET;
1001+
}
1002+
9961003
static int check_pkt_ptr_alignment(const struct bpf_reg_state *reg,
9971004
int off, int size, bool strict)
9981005
{
@@ -1050,10 +1057,10 @@ static int check_generic_ptr_alignment(const struct bpf_reg_state *reg,
10501057
}
10511058

10521059
static int check_ptr_alignment(struct bpf_verifier_env *env,
1053-
const struct bpf_reg_state *reg,
1054-
int off, int size)
1060+
const struct bpf_reg_state *reg, int off,
1061+
int size, bool strict_alignment_once)
10551062
{
1056-
bool strict = env->strict_alignment;
1063+
bool strict = env->strict_alignment || strict_alignment_once;
10571064
const char *pointer_desc = "";
10581065

10591066
switch (reg->type) {
@@ -1109,9 +1116,9 @@ static void coerce_reg_to_size(struct bpf_reg_state *reg, int size)
11091116
* if t==write && value_regno==-1, some unknown value is stored into memory
11101117
* if t==read && value_regno==-1, don't care what we read from memory
11111118
*/
1112-
static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regno, int off,
1113-
int bpf_size, enum bpf_access_type t,
1114-
int value_regno)
1119+
static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regno,
1120+
int off, int bpf_size, enum bpf_access_type t,
1121+
int value_regno, bool strict_alignment_once)
11151122
{
11161123
struct bpf_verifier_state *state = &env->cur_state;
11171124
struct bpf_reg_state *reg = &state->regs[regno];
@@ -1122,7 +1129,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
11221129
return size;
11231130

11241131
/* alignment checks will add in reg->off themselves */
1125-
err = check_ptr_alignment(env, reg, off, size);
1132+
err = check_ptr_alignment(env, reg, off, size, strict_alignment_once);
11261133
if (err)
11271134
return err;
11281135

@@ -1265,21 +1272,23 @@ static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_ins
12651272
return -EACCES;
12661273
}
12671274

1268-
if (is_ctx_reg(env, insn->dst_reg)) {
1269-
verbose("BPF_XADD stores into R%d context is not allowed\n",
1270-
insn->dst_reg);
1275+
if (is_ctx_reg(env, insn->dst_reg) ||
1276+
is_pkt_reg(env, insn->dst_reg)) {
1277+
verbose("BPF_XADD stores into R%d %s is not allowed\n",
1278+
insn->dst_reg, is_ctx_reg(env, insn->dst_reg) ?
1279+
"context" : "packet");
12711280
return -EACCES;
12721281
}
12731282

12741283
/* check whether atomic_add can read the memory */
12751284
err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
1276-
BPF_SIZE(insn->code), BPF_READ, -1);
1285+
BPF_SIZE(insn->code), BPF_READ, -1, true);
12771286
if (err)
12781287
return err;
12791288

12801289
/* check whether atomic_add can write into the same memory */
12811290
return check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
1282-
BPF_SIZE(insn->code), BPF_WRITE, -1);
1291+
BPF_SIZE(insn->code), BPF_WRITE, -1, true);
12831292
}
12841293

12851294
/* Does this register contain a constant zero? */
@@ -1735,7 +1744,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
17351744
* is inferred from register state.
17361745
*/
17371746
for (i = 0; i < meta.access_size; i++) {
1738-
err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B, BPF_WRITE, -1);
1747+
err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B,
1748+
BPF_WRITE, -1, false);
17391749
if (err)
17401750
return err;
17411751
}
@@ -3801,7 +3811,7 @@ static int do_check(struct bpf_verifier_env *env)
38013811
*/
38023812
err = check_mem_access(env, insn_idx, insn->src_reg, insn->off,
38033813
BPF_SIZE(insn->code), BPF_READ,
3804-
insn->dst_reg);
3814+
insn->dst_reg, false);
38053815
if (err)
38063816
return err;
38073817

@@ -3853,7 +3863,7 @@ static int do_check(struct bpf_verifier_env *env)
38533863
/* check that memory (dst_reg + off) is writeable */
38543864
err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
38553865
BPF_SIZE(insn->code), BPF_WRITE,
3856-
insn->src_reg);
3866+
insn->src_reg, false);
38573867
if (err)
38583868
return err;
38593869

@@ -3888,7 +3898,7 @@ static int do_check(struct bpf_verifier_env *env)
38883898
/* check that memory (dst_reg + off) is writeable */
38893899
err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
38903900
BPF_SIZE(insn->code), BPF_WRITE,
3891-
-1);
3901+
-1, false);
38923902
if (err)
38933903
return err;
38943904

‎tools/testing/selftests/bpf/test_verifier.c

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7880,6 +7880,64 @@ static struct bpf_test tests[] = {
78807880
.prog_type = BPF_PROG_TYPE_XDP,
78817881
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
78827882
},
7883+
{
7884+
"xadd/w check unaligned stack",
7885+
.insns = {
7886+
BPF_MOV64_IMM(BPF_REG_0, 1),
7887+
BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_0, -8),
7888+
BPF_STX_XADD(BPF_W, BPF_REG_10, BPF_REG_0, -7),
7889+
BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
7890+
BPF_EXIT_INSN(),
7891+
},
7892+
.result = REJECT,
7893+
.errstr = "misaligned stack access off",
7894+
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
7895+
},
7896+
{
7897+
"xadd/w check unaligned map",
7898+
.insns = {
7899+
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
7900+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
7901+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
7902+
BPF_LD_MAP_FD(BPF_REG_1, 0),
7903+
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
7904+
BPF_FUNC_map_lookup_elem),
7905+
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
7906+
BPF_EXIT_INSN(),
7907+
BPF_MOV64_IMM(BPF_REG_1, 1),
7908+
BPF_STX_XADD(BPF_W, BPF_REG_0, BPF_REG_1, 3),
7909+
BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, 3),
7910+
BPF_EXIT_INSN(),
7911+
},
7912+
.fixup_map1 = { 3 },
7913+
.result = REJECT,
7914+
.errstr = "misaligned value access off",
7915+
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
7916+
},
7917+
{
7918+
"xadd/w check unaligned pkt",
7919+
.insns = {
7920+
BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
7921+
offsetof(struct xdp_md, data)),
7922+
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
7923+
offsetof(struct xdp_md, data_end)),
7924+
BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
7925+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
7926+
BPF_JMP_REG(BPF_JLT, BPF_REG_1, BPF_REG_3, 2),
7927+
BPF_MOV64_IMM(BPF_REG_0, 99),
7928+
BPF_JMP_IMM(BPF_JA, 0, 0, 6),
7929+
BPF_MOV64_IMM(BPF_REG_0, 1),
7930+
BPF_ST_MEM(BPF_W, BPF_REG_2, 0, 0),
7931+
BPF_ST_MEM(BPF_W, BPF_REG_2, 3, 0),
7932+
BPF_STX_XADD(BPF_W, BPF_REG_2, BPF_REG_0, 1),
7933+
BPF_STX_XADD(BPF_W, BPF_REG_2, BPF_REG_0, 2),
7934+
BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_2, 1),
7935+
BPF_EXIT_INSN(),
7936+
},
7937+
.result = REJECT,
7938+
.errstr = "BPF_XADD stores into R2 packet",
7939+
.prog_type = BPF_PROG_TYPE_XDP,
7940+
},
78837941
};
78847942

78857943
static int probe_filter_length(const struct bpf_insn *fp)

0 commit comments

Comments
 (0)