On Wed, Sep 01, 2021 at 01:31:18PM +0200, Greg KH wrote:
On Wed, Sep 01, 2021 at 01:30:37PM +0200, Greg KH wrote:
On Mon, Aug 30, 2021 at 03:32:08PM -0300, Thadeu Lima de Souza Cascardo wrote:
From: Daniel Borkmann daniel@iogearbox.net
Partially undo old commit 144cd91c4c2b ("bpf: move tmp variable into ax register in interpreter"). The reason we need this here is because ax register will be used for holding temporary state for div/mod instruction which otherwise interpreter would corrupt. This will cause a small +8 byte stack increase for interpreter, but with the gain that we can use it from verifier rewrites as scratch register.
Signed-off-by: Daniel Borkmann daniel@iogearbox.net Reviewed-by: John Fastabend john.fastabend@gmail.com [cascardo: This partial revert is needed in order to support using AX for the following two commits, as there is no JMP32 on 4.19.y] Signed-off-by: Thadeu Lima de Souza Cascardo cascardo@canonical.com
kernel/bpf/core.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index e7211b0fa27c..30d871be9974 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -616,9 +616,6 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from, * below. * * Constant blinding is only used by JITs, not in the interpreter.
* The interpreter uses AX in some occasions as a local temporary* register e.g. in DIV or MOD instructions.*
- In restricted circumstances, the verifier can also use the AX
- register for rewrites as long as they do not interfere with
- the above cases!
@@ -951,6 +948,7 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u32 tail_call_cnt = 0; void *ptr; int off;
- u64 tmp;
#define CONT ({ insn++; goto select_insn; }) #define CONT_JMP ({ insn++; goto select_insn; }) @@ -1013,22 +1011,22 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, ALU64_MOD_X: if (unlikely(SRC == 0)) return 0;
div64_u64_rem(DST, SRC, &AX);DST = AX;
div64_u64_rem(DST, SRC, &tmp);DST = tmp;
AX = (u32) DST;DST = do_div(AX, (u32) SRC);
tmp = (u32) DST;DST = do_div(tmp, (u32) SRC);
div64_u64_rem(DST, IMM, &AX);DST = AX;
div64_u64_rem(DST, IMM, &tmp);DST = tmp;
AX = (u32) DST;DST = do_div(AX, (u32) IMM);
tmp = (u32) DST;DST = do_div(tmp, (u32) IMM);@@ -1038,17 +1036,17 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, ALU_DIV_X: if (unlikely((u32)SRC == 0)) return 0;
AX = (u32) DST;do_div(AX, (u32) SRC);DST = (u32) AX;
tmp = (u32) DST;do_div(tmp, (u32) SRC);DST = (u32) tmp;
AX = (u32) DST;do_div(AX, (u32) IMM);DST = (u32) AX;
tmp = (u32) DST;do_div(tmp, (u32) IMM);DST = (u32) tmp;-- 2.30.2
Oops, no, this patch causes build errors:
kernel/bpf/core.c: In function ‘___bpf_prog_run’: kernel/bpf/core.c:951:13: error: redeclaration of ‘tmp’ with no linkage 951 | u64 tmp; | ^~~ kernel/bpf/core.c:839:13: note: previous declaration of ‘tmp’ with type ‘u64’ {aka ‘long long unsigned int’} 839 | u64 tmp; | ^~~ make[2]: *** [scripts/Makefile.build:329: kernel/bpf/core.o] Error 1
Please fix up and resend the whole series, as I will go drop these 3 patches from the 4.14.y queue now.
All _4_ patches I mean. now dropped...
Ah... it seems I only built it with CONFIG_BPF_JIT_ALWAYS_ON. I will build with both that option on and off and check the results.
Thanks for catching this.
Cascardo.