Skip to content

Commit e0ee9c1

Browse files
Alexei Starovoitovdavem330
Alexei Starovoitov
authored andcommitted
x86: bpf_jit: fix two bugs in eBPF JIT compiler
1. JIT compiler using multi-pass approach to converge to final image size, since x86 instructions are variable length. It starts with large gaps between instructions (so some jumps may use imm32 instead of imm8) and iterates until total program size is the same as in previous pass. This algorithm works only if program size is strictly decreasing. Programs that use LD_ABS insn need additional code in prologue, but it was not emitted during 1st pass, so there was a chance that 2nd pass would adjust imm32->imm8 jump offsets to the same number of bytes as increase in prologue, which may cause algorithm to erroneously decide that size converged. Fix it by always emitting largest prologue in the first pass which is detected by oldproglen==0 check. Also change error check condition 'proglen != oldproglen' to fail gracefully. 2. while staring at the code realized that 64-byte buffer may not be enough when 1st insn is large, so increase it to 128 to avoid buffer overflow (theoretical maximum size of prologue+div is 109) and add runtime check. Fixes: 6225827 ("net: filter: x86: internal BPF JIT") Reported-by: Darrick J. Wong <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Tested-by: Darrick J. Wong <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent b2532eb commit e0ee9c1

File tree

1 file changed

+19
-6
lines changed

1 file changed

+19
-6
lines changed

arch/x86/net/bpf_jit_comp.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,17 @@ struct jit_context {
182182
bool seen_ld_abs;
183183
};
184184

185+
/* maximum number of bytes emitted while JITing one eBPF insn */
186+
#define BPF_MAX_INSN_SIZE 128
187+
#define BPF_INSN_SAFETY 64
188+
185189
static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
186190
int oldproglen, struct jit_context *ctx)
187191
{
188192
struct bpf_insn *insn = bpf_prog->insnsi;
189193
int insn_cnt = bpf_prog->len;
190-
u8 temp[64];
194+
bool seen_ld_abs = ctx->seen_ld_abs | (oldproglen == 0);
195+
u8 temp[BPF_MAX_INSN_SIZE + BPF_INSN_SAFETY];
191196
int i;
192197
int proglen = 0;
193198
u8 *prog = temp;
@@ -225,7 +230,7 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
225230
EMIT2(0x31, 0xc0); /* xor eax, eax */
226231
EMIT3(0x4D, 0x31, 0xED); /* xor r13, r13 */
227232

228-
if (ctx->seen_ld_abs) {
233+
if (seen_ld_abs) {
229234
/* r9d : skb->len - skb->data_len (headlen)
230235
* r10 : skb->data
231236
*/
@@ -685,7 +690,7 @@ xadd: if (is_imm8(insn->off))
685690
case BPF_JMP | BPF_CALL:
686691
func = (u8 *) __bpf_call_base + imm32;
687692
jmp_offset = func - (image + addrs[i]);
688-
if (ctx->seen_ld_abs) {
693+
if (seen_ld_abs) {
689694
EMIT2(0x41, 0x52); /* push %r10 */
690695
EMIT2(0x41, 0x51); /* push %r9 */
691696
/* need to adjust jmp offset, since
@@ -699,7 +704,7 @@ xadd: if (is_imm8(insn->off))
699704
return -EINVAL;
700705
}
701706
EMIT1_off32(0xE8, jmp_offset);
702-
if (ctx->seen_ld_abs) {
707+
if (seen_ld_abs) {
703708
EMIT2(0x41, 0x59); /* pop %r9 */
704709
EMIT2(0x41, 0x5A); /* pop %r10 */
705710
}
@@ -804,7 +809,8 @@ xadd: if (is_imm8(insn->off))
804809
goto common_load;
805810
case BPF_LD | BPF_ABS | BPF_W:
806811
func = CHOOSE_LOAD_FUNC(imm32, sk_load_word);
807-
common_load: ctx->seen_ld_abs = true;
812+
common_load:
813+
ctx->seen_ld_abs = seen_ld_abs = true;
808814
jmp_offset = func - (image + addrs[i]);
809815
if (!func || !is_simm32(jmp_offset)) {
810816
pr_err("unsupported bpf func %d addr %p image %p\n",
@@ -878,6 +884,11 @@ common_load: ctx->seen_ld_abs = true;
878884
}
879885

880886
ilen = prog - temp;
887+
if (ilen > BPF_MAX_INSN_SIZE) {
888+
pr_err("bpf_jit_compile fatal insn size error\n");
889+
return -EFAULT;
890+
}
891+
881892
if (image) {
882893
if (unlikely(proglen + ilen > oldproglen)) {
883894
pr_err("bpf_jit_compile fatal error\n");
@@ -934,9 +945,11 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
934945
goto out;
935946
}
936947
if (image) {
937-
if (proglen != oldproglen)
948+
if (proglen != oldproglen) {
938949
pr_err("bpf_jit: proglen=%d != oldproglen=%d\n",
939950
proglen, oldproglen);
951+
goto out;
952+
}
940953
break;
941954
}
942955
if (proglen == oldproglen) {

0 commit comments

Comments
 (0)