Skip to content

Commit 7dd34d7

Browse files
Yonghong SongAlexei Starovoitov
Yonghong Song
authored and
Alexei Starovoitov
committed
bpf: Fix a sdiv overflow issue
Zac Ecob reported a problem where a bpf program may cause kernel crash due to the following error: Oops: divide error: 0000 [#1] PREEMPT SMP KASAN PTI The failure is due to the below signed divide: LLONG_MIN/-1 where LLONG_MIN equals to -9,223,372,036,854,775,808. LLONG_MIN/-1 is supposed to give a positive number 9,223,372,036,854,775,808, but it is impossible since for 64-bit system, the maximum positive number is 9,223,372,036,854,775,807. On x86_64, LLONG_MIN/-1 will cause a kernel exception. On arm64, the result for LLONG_MIN/-1 is LLONG_MIN. Further investigation found all the following sdiv/smod cases may trigger an exception when bpf program is running on x86_64 platform: - LLONG_MIN/-1 for 64bit operation - INT_MIN/-1 for 32bit operation - LLONG_MIN%-1 for 64bit operation - INT_MIN%-1 for 32bit operation where -1 can be an immediate or in a register. On arm64, there are no exceptions: - LLONG_MIN/-1 = LLONG_MIN - INT_MIN/-1 = INT_MIN - LLONG_MIN%-1 = 0 - INT_MIN%-1 = 0 where -1 can be an immediate or in a register. Insn patching is needed to handle the above cases and the patched codes produced results aligned with above arm64 result. The below are pseudo codes to handle sdiv/smod exceptions including both divisor -1 and divisor 0 and the divisor is stored in a register. sdiv: tmp = rX tmp += 1 /* [-1, 0] -> [0, 1] if tmp >(unsigned) 1 goto L2 if tmp == 0 goto L1 rY = 0 L1: rY = -rY; goto L3 L2: rY /= rX L3: smod: tmp = rX tmp += 1 /* [-1, 0] -> [0, 1] if tmp >(unsigned) 1 goto L1 if tmp == 1 (is64 ? goto L2 : goto L3) rY = 0; goto L2 L1: rY %= rX L2: goto L4 // only when !is64 L3: wY = wY // only when !is64 L4: [1] https://lore.kernel.org/bpf/tPJLTEh7S_DxFEqAI2Ji5MBSoZVg7_G-Py2iaZpAaWtM961fFTWtsnlzwvTbzBzaUzwQAoNATXKUlt0LZOFgnDcIyKCswAnAGdUF3LBrhGQ=@protonmail.com/ Reported-by: Zac Ecob <[email protected]> Signed-off-by: Yonghong Song <[email protected]> Acked-by: Andrii Nakryiko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent ea02a94 commit 7dd34d7

File tree

1 file changed

+89
-4
lines changed

1 file changed

+89
-4
lines changed

‎kernel/bpf/verifier.c

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20499,13 +20499,46 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2049920499
/* Convert BPF_CLASS(insn->code) == BPF_ALU64 to 32-bit ALU */
2050020500
insn->code = BPF_ALU | BPF_OP(insn->code) | BPF_SRC(insn->code);
2050120501

20502-
/* Make divide-by-zero exceptions impossible. */
20502+
/* Make sdiv/smod divide-by-minus-one exceptions impossible. */
20503+
if ((insn->code == (BPF_ALU64 | BPF_MOD | BPF_K) ||
20504+
insn->code == (BPF_ALU64 | BPF_DIV | BPF_K) ||
20505+
insn->code == (BPF_ALU | BPF_MOD | BPF_K) ||
20506+
insn->code == (BPF_ALU | BPF_DIV | BPF_K)) &&
20507+
insn->off == 1 && insn->imm == -1) {
20508+
bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
20509+
bool isdiv = BPF_OP(insn->code) == BPF_DIV;
20510+
struct bpf_insn *patchlet;
20511+
struct bpf_insn chk_and_sdiv[] = {
20512+
BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) |
20513+
BPF_NEG | BPF_K, insn->dst_reg,
20514+
0, 0, 0),
20515+
};
20516+
struct bpf_insn chk_and_smod[] = {
20517+
BPF_MOV32_IMM(insn->dst_reg, 0),
20518+
};
20519+
20520+
patchlet = isdiv ? chk_and_sdiv : chk_and_smod;
20521+
cnt = isdiv ? ARRAY_SIZE(chk_and_sdiv) : ARRAY_SIZE(chk_and_smod);
20522+
20523+
new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
20524+
if (!new_prog)
20525+
return -ENOMEM;
20526+
20527+
delta += cnt - 1;
20528+
env->prog = prog = new_prog;
20529+
insn = new_prog->insnsi + i + delta;
20530+
goto next_insn;
20531+
}
20532+
20533+
/* Make divide-by-zero and divide-by-minus-one exceptions impossible. */
2050320534
if (insn->code == (BPF_ALU64 | BPF_MOD | BPF_X) ||
2050420535
insn->code == (BPF_ALU64 | BPF_DIV | BPF_X) ||
2050520536
insn->code == (BPF_ALU | BPF_MOD | BPF_X) ||
2050620537
insn->code == (BPF_ALU | BPF_DIV | BPF_X)) {
2050720538
bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
2050820539
bool isdiv = BPF_OP(insn->code) == BPF_DIV;
20540+
bool is_sdiv = isdiv && insn->off == 1;
20541+
bool is_smod = !isdiv && insn->off == 1;
2050920542
struct bpf_insn *patchlet;
2051020543
struct bpf_insn chk_and_div[] = {
2051120544
/* [R,W]x div 0 -> 0 */
@@ -20525,10 +20558,62 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2052520558
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
2052620559
BPF_MOV32_REG(insn->dst_reg, insn->dst_reg),
2052720560
};
20561+
struct bpf_insn chk_and_sdiv[] = {
20562+
/* [R,W]x sdiv 0 -> 0
20563+
* LLONG_MIN sdiv -1 -> LLONG_MIN
20564+
* INT_MIN sdiv -1 -> INT_MIN
20565+
*/
20566+
BPF_MOV64_REG(BPF_REG_AX, insn->src_reg),
20567+
BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) |
20568+
BPF_ADD | BPF_K, BPF_REG_AX,
20569+
0, 0, 1),
20570+
BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
20571+
BPF_JGT | BPF_K, BPF_REG_AX,
20572+
0, 4, 1),
20573+
BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
20574+
BPF_JEQ | BPF_K, BPF_REG_AX,
20575+
0, 1, 0),
20576+
BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) |
20577+
BPF_MOV | BPF_K, insn->dst_reg,
20578+
0, 0, 0),
20579+
/* BPF_NEG(LLONG_MIN) == -LLONG_MIN == LLONG_MIN */
20580+
BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) |
20581+
BPF_NEG | BPF_K, insn->dst_reg,
20582+
0, 0, 0),
20583+
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
20584+
*insn,
20585+
};
20586+
struct bpf_insn chk_and_smod[] = {
20587+
/* [R,W]x mod 0 -> [R,W]x */
20588+
/* [R,W]x mod -1 -> 0 */
20589+
BPF_MOV64_REG(BPF_REG_AX, insn->src_reg),
20590+
BPF_RAW_INSN((is64 ? BPF_ALU64 : BPF_ALU) |
20591+
BPF_ADD | BPF_K, BPF_REG_AX,
20592+
0, 0, 1),
20593+
BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
20594+
BPF_JGT | BPF_K, BPF_REG_AX,
20595+
0, 3, 1),
20596+
BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
20597+
BPF_JEQ | BPF_K, BPF_REG_AX,
20598+
0, 3 + (is64 ? 0 : 1), 1),
20599+
BPF_MOV32_IMM(insn->dst_reg, 0),
20600+
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
20601+
*insn,
20602+
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
20603+
BPF_MOV32_REG(insn->dst_reg, insn->dst_reg),
20604+
};
2052820605

20529-
patchlet = isdiv ? chk_and_div : chk_and_mod;
20530-
cnt = isdiv ? ARRAY_SIZE(chk_and_div) :
20531-
ARRAY_SIZE(chk_and_mod) - (is64 ? 2 : 0);
20606+
if (is_sdiv) {
20607+
patchlet = chk_and_sdiv;
20608+
cnt = ARRAY_SIZE(chk_and_sdiv);
20609+
} else if (is_smod) {
20610+
patchlet = chk_and_smod;
20611+
cnt = ARRAY_SIZE(chk_and_smod) - (is64 ? 2 : 0);
20612+
} else {
20613+
patchlet = isdiv ? chk_and_div : chk_and_mod;
20614+
cnt = isdiv ? ARRAY_SIZE(chk_and_div) :
20615+
ARRAY_SIZE(chk_and_mod) - (is64 ? 2 : 0);
20616+
}
2053220617

2053320618
new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
2053420619
if (!new_prog)

0 commit comments

Comments
 (0)