linux-kernel - bpf: incorrect value spill in check_stack_write_fixed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACkBjsYXA8myxoP0Naz=ZxB0FWG-xS9e28CSFffGk1bA_n5RXw@mail.gmail.com>
Date:   Wed, 25 Oct 2023 11:16:34 +0200
From:   Hao Sun <sunhao.th@...il.com>
To:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>,
        Yonghong Song <yonghong.song@...ux.dev>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>
Cc:     bpf <bpf@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: bpf: incorrect value spill in check_stack_write_fixed_off()

Hi,

In check_stack_write_fixed_off(), the verifier creates a fake reg to store the
imm in a BPF_ST_MEM:
...
else if (!reg && !(off % BPF_REG_SIZE) && is_bpf_st_mem(insn) &&
insn->imm != 0 && env->bpf_capable) {
        struct bpf_reg_state fake_reg = {};

        __mark_reg_known(&fake_reg, (u32)insn->imm);
        fake_reg.type = SCALAR_VALUE;
        save_register_state(state, spi, &fake_reg, size);

Here, insn->imm is cast to u32, and used to mark fake_reg, which is incorrect
and may lose sign information. Consider the following program:

r2 = r10
*(u64*)(r2 -40) = -44
r0 = *(u64*)(r2 - 40)
if r0 s<= 0xa goto +2
r0 = 0
exit
r0  = 1
exit

The verifier gives the following log:

-------- Verifier Log --------
func#0 @0
0: R1=ctx(off=0,imm=0) R10=fp0
0: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
1: (7a) *(u64 *)(r2 -40) = -44        ; R2_w=fp0 fp-40_w=4294967252
2: (79) r0 = *(u64 *)(r2 -40)         ; R0_w=4294967252 R2_w=fp0
fp-40_w=4294967252
3: (c5) if r0 s< 0xa goto pc+2
mark_precise: frame0: last_idx 3 first_idx 0 subseq_idx -1
mark_precise: frame0: regs=r0 stack= before 2: (79) r0 = *(u64 *)(r2 -40)
3: R0_w=4294967252
4: (b7) r0 = 1                        ; R0_w=1
5: (95) exit
verification time 7971 usec
stack depth 40
processed 6 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0

Here, the verifier incorrectly thinks R0 is 0xffffffd4, which should
be 0xffffffffffffffd4,
due to the u32 cast in check_stack_write_fixed_off(). This makes the verifier
collect incorrect reg scalar range.

Since insn->imm is i32, we should cast it to the signed integer with
correct size
according to BPF_MEM, then promoting the imm to u64 to mark fake reg as
known, right?

Best
Hao