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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 27 May 2020 09:58:17 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Alexander Potapenko <glider@...gle.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Michal Kubecek <mkubecek@...e.cz>,
        Alexei Starovoitov <ast@...nel.org>,
        Dmitriy Vyukov <dvyukov@...gle.com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: Self-XORing BPF registers is undefined behavior

On Wed, May 27, 2020 at 8:52 AM Alexander Potapenko <glider@...gle.com> wrote:
>
> This basically means that BPF's output register was uninitialized when
> ___bpf_prog_run() returned.
>
> When I replace the lines initializing registers A and X in net/core/filter.c:
>
> -               *new_insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_A, BPF_REG_A);
> -               *new_insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_X, BPF_REG_X);
>
> with
>
> +               *new_insn++ = BPF_MOV32_IMM(BPF_REG_A, 0);
> +               *new_insn++ = BPF_MOV32_IMM(BPF_REG_X, 0);
>
> , the bug goes away, therefore I think it's being caused by XORing the
> initially uninitialized registers with themselves.
>
> kernel/bpf/core.c:1408, where the uninitialized value was stored to
> memory, points to the "ALU(ADD,  +)" macro in ___bpf_prog_run().
> But the debug info seems to be incorrect here: if I comment this line
> out and unroll the macro manually, KMSAN points to "ALU(SUB,  -)".
> Most certainly it's actually one of the XOR instruction declarations.
>
> Do you think it makes sense to use the UB-proof BPF_MOV32_IMM
> instructions to initialize the registers?

I think it's better for UBsan to get smarter about xor-ing.

Powered by blists - more mailing lists