From: Yonghong Song <yonghong.song@linux.dev>
To: kernel test robot <lkp@intel.com>, bpf@vger.kernel.org
Cc: oe-kbuild-all@lists.linux.dev,
Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>,
Zac Ecob <zacecob@protonmail.com>
Subject: Re: [PATCH bpf-next 1/2] bpf: Fix a sdiv overflow issue
Date: Thu, 12 Sep 2024 09:43:09 -0700 [thread overview]
Message-ID: <b327cfaf-97d7-4d7a-9d74-27927ef564ee@linux.dev> (raw)
In-Reply-To: <202409121439.L01ZquSs-lkp@intel.com>
On 9/11/24 11:54 PM, kernel test robot wrote:
> Hi Yonghong,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on bpf-next/master]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Yonghong-Song/selftests-bpf-Add-a-couple-of-tests-for-potential-sdiv-overflow/20240911-124236
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/20240911044017.2261738-1-yonghong.song%40linux.dev
> patch subject: [PATCH bpf-next 1/2] bpf: Fix a sdiv overflow issue
> config: x86_64-randconfig-121-20240912 (https://download.01.org/0day-ci/archive/20240912/202409121439.L01ZquSs-lkp@intel.com/config)
> compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240912/202409121439.L01ZquSs-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409121439.L01ZquSs-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
> kernel/bpf/verifier.c:21184:38: sparse: sparse: subtraction of functions? Share your drugs
> kernel/bpf/verifier.c: note: in included file (through include/linux/bpf.h, include/linux/bpf-cgroup.h):
> include/linux/bpfptr.h:65:40: sparse: sparse: cast to non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast from non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast to non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast from non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast to non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast from non-scalar
>>> kernel/bpf/verifier.c:20538:33: sparse: sparse: cast truncates bits from constant value (8000000000000000 becomes 0)
>
The above is expected. See below macro definition in include/linux/filter.h
/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
#define BPF_LD_IMM64(DST, IMM) \
BPF_LD_IMM64_RAW(DST, 0, IMM)
#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
((struct bpf_insn) { \
.code = BPF_LD | BPF_DW | BPF_IMM, \
.dst_reg = DST, \
.src_reg = SRC, \
.off = 0, \
.imm = (__u32) (IMM) }), \
((struct bpf_insn) { \
.code = 0, /* zero is reserved opcode */ \
.dst_reg = 0, \
.src_reg = 0, \
.off = 0, \
.imm = ((__u64) (IMM)) >> 32 })
So (__u32) (IMM) will cause a truncation and may cause a warning,
but it is expected for bpf.
> include/linux/bpfptr.h:65:40: sparse: sparse: cast to non-scalar
> include/linux/bpfptr.h:65:40: sparse: sparse: cast from non-scalar
>
> vim +20538 kernel/bpf/verifier.c
>
> 20445
> 20446 /* Do various post-verification rewrites in a single program pass.
> 20447 * These rewrites simplify JIT and interpreter implementations.
> 20448 */
> 20449 static int do_misc_fixups(struct bpf_verifier_env *env)
> 20450 {
> 20451 struct bpf_prog *prog = env->prog;
> 20452 enum bpf_attach_type eatype = prog->expected_attach_type;
> 20453 enum bpf_prog_type prog_type = resolve_prog_type(prog);
> 20454 struct bpf_insn *insn = prog->insnsi;
> 20455 const struct bpf_func_proto *fn;
> 20456 const int insn_cnt = prog->len;
> 20457 const struct bpf_map_ops *ops;
> 20458 struct bpf_insn_aux_data *aux;
> 20459 struct bpf_insn *insn_buf = env->insn_buf;
> 20460 struct bpf_prog *new_prog;
> 20461 struct bpf_map *map_ptr;
> 20462 int i, ret, cnt, delta = 0, cur_subprog = 0;
> 20463 struct bpf_subprog_info *subprogs = env->subprog_info;
> 20464 u16 stack_depth = subprogs[cur_subprog].stack_depth;
> 20465 u16 stack_depth_extra = 0;
> 20466
> 20467 if (env->seen_exception && !env->exception_callback_subprog) {
> 20468 struct bpf_insn patch[] = {
> 20469 env->prog->insnsi[insn_cnt - 1],
> 20470 BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
> 20471 BPF_EXIT_INSN(),
> 20472 };
> 20473
> 20474 ret = add_hidden_subprog(env, patch, ARRAY_SIZE(patch));
> 20475 if (ret < 0)
> 20476 return ret;
> 20477 prog = env->prog;
> 20478 insn = prog->insnsi;
> 20479
> 20480 env->exception_callback_subprog = env->subprog_cnt - 1;
> 20481 /* Don't update insn_cnt, as add_hidden_subprog always appends insns */
> 20482 mark_subprog_exc_cb(env, env->exception_callback_subprog);
> 20483 }
> 20484
> 20485 for (i = 0; i < insn_cnt;) {
> 20486 if (insn->code == (BPF_ALU64 | BPF_MOV | BPF_X) && insn->imm) {
> 20487 if ((insn->off == BPF_ADDR_SPACE_CAST && insn->imm == 1) ||
> 20488 (((struct bpf_map *)env->prog->aux->arena)->map_flags & BPF_F_NO_USER_CONV)) {
> 20489 /* convert to 32-bit mov that clears upper 32-bit */
> 20490 insn->code = BPF_ALU | BPF_MOV | BPF_X;
> 20491 /* clear off and imm, so it's a normal 'wX = wY' from JIT pov */
> 20492 insn->off = 0;
> 20493 insn->imm = 0;
> 20494 } /* cast from as(0) to as(1) should be handled by JIT */
> 20495 goto next_insn;
> 20496 }
> 20497
> 20498 if (env->insn_aux_data[i + delta].needs_zext)
> 20499 /* Convert BPF_CLASS(insn->code) == BPF_ALU64 to 32-bit ALU */
> 20500 insn->code = BPF_ALU | BPF_OP(insn->code) | BPF_SRC(insn->code);
> 20501
> 20502 /* Make divide-by-zero exceptions impossible. */
> 20503 if (insn->code == (BPF_ALU64 | BPF_MOD | BPF_X) ||
> 20504 insn->code == (BPF_ALU64 | BPF_DIV | BPF_X) ||
> 20505 insn->code == (BPF_ALU | BPF_MOD | BPF_X) ||
> 20506 insn->code == (BPF_ALU | BPF_DIV | BPF_X)) {
> 20507 bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
> 20508 bool isdiv = BPF_OP(insn->code) == BPF_DIV;
> 20509 bool is_sdiv64 = is64 && isdiv && insn->off == 1;
> 20510 struct bpf_insn *patchlet;
> 20511 struct bpf_insn chk_and_div[] = {
> 20512 /* [R,W]x div 0 -> 0 */
> 20513 BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
> 20514 BPF_JNE | BPF_K, insn->src_reg,
> 20515 0, 2, 0),
> 20516 BPF_ALU32_REG(BPF_XOR, insn->dst_reg, insn->dst_reg),
> 20517 BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> 20518 *insn,
> 20519 };
> 20520 struct bpf_insn chk_and_mod[] = {
> 20521 /* [R,W]x mod 0 -> [R,W]x */
> 20522 BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
> 20523 BPF_JEQ | BPF_K, insn->src_reg,
> 20524 0, 1 + (is64 ? 0 : 1), 0),
> 20525 *insn,
> 20526 BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> 20527 BPF_MOV32_REG(insn->dst_reg, insn->dst_reg),
> 20528 };
> 20529 struct bpf_insn chk_and_sdiv64[] = {
> 20530 /* Rx sdiv 0 -> 0 */
> 20531 BPF_RAW_INSN(BPF_JMP | BPF_JNE | BPF_K, insn->src_reg,
> 20532 0, 2, 0),
> 20533 BPF_ALU32_REG(BPF_XOR, insn->dst_reg, insn->dst_reg),
> 20534 BPF_JMP_IMM(BPF_JA, 0, 0, 8),
> 20535 /* LLONG_MIN sdiv -1 -> LLONG_MIN */
> 20536 BPF_RAW_INSN(BPF_JMP | BPF_JNE | BPF_K, insn->src_reg,
> 20537 0, 6, -1),
> 20538 BPF_LD_IMM64(insn->src_reg, LLONG_MIN),
the warning is triggered here.
> 20539 BPF_RAW_INSN(BPF_JMP | BPF_JNE | BPF_X, insn->dst_reg,
> 20540 insn->src_reg, 2, 0),
> 20541 BPF_MOV64_IMM(insn->src_reg, -1),
> 20542 BPF_JMP_IMM(BPF_JA, 0, 0, 2),
> 20543 BPF_MOV64_IMM(insn->src_reg, -1),
> 20544 *insn,
> 20545 };
> 20546
[...]
prev parent reply other threads:[~2024-09-12 16:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-11 4:40 [PATCH bpf-next 1/2] bpf: Fix a sdiv overflow issue Yonghong Song
2024-09-11 4:40 ` [PATCH bpf-next 2/2] selftests/bpf: Add a couple of tests for potential sdiv overflow Yonghong Song
2024-09-11 14:18 ` [PATCH bpf-next 1/2] bpf: Fix a sdiv overflow issue Daniel Borkmann
2024-09-11 15:14 ` Yonghong Song
2024-09-11 15:52 ` Alexei Starovoitov
2024-09-11 17:01 ` Yonghong Song
2024-09-11 17:17 ` Andrii Nakryiko
2024-09-11 17:32 ` Yonghong Song
2024-09-12 6:54 ` kernel test robot
2024-09-12 16:43 ` Yonghong Song [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b327cfaf-97d7-4d7a-9d74-27927ef564ee@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=lkp@intel.com \
--cc=martin.lau@kernel.org \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=zacecob@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.