netdev - Re: Self-XORing BPF registers is undefined behavior

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Dec 2018 12:23:11 +0100
From:   Alexander Potapenko <glider@...gle.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     ast@...nel.org, daniel@...earbox.net,
        Dmitriy Vyukov <dvyukov@...gle.com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: Self-XORing BPF registers is undefined behavior

On Thu, Dec 13, 2018 at 12:06 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
>
> On 12/13/2018 03:00 AM, Alexander Potapenko wrote:
> > Hi BPF maintainers,
> >
> > some time ago KMSAN found an issue in BPF code which we decided to
> > suppress at that point, but now I'd like to bring it to your
> > attention.
> > Namely, some BPF programs may contain instructions that XOR a register
> > with itself.
> > This effectively results in the following C code:
> >   regs[BPF_REG_A] = regs[BPF_REG_A] ^ regs[BPF_REG_A];
> > or
> >   regs[BPF_REG_X] = regs[BPF_REG_X] ^ regs[BPF_REG_X];
> > being executed.
> >
> > According to the C11 standard this is undefined behavior, so KMSAN
> > reports an error in this case.
>
> eBPF is not C11 ;)
And is planning to stay such forever? :)
I'm not a language lawyer, so I can't tell for sure if this is valid
in C99 either. I think the term "trap representation" had already been
there.

> XOR boolean operation on a cpu is following boolean logic, which is much stronger than
> any C standard.
Yes, this is true if we compile a eBPF program into assembly code.
But if the JIT is off, it ends up being interpreted by
___bpf_prog_run(), which just executes C code from a big switch:
...
  ALU_XOR_X:
    regs[insn->dst_reg] = regs[insn->dst_reg] ^ regs[insn->src_reg];
...

Note that it's even unknown at compile time that dst_reg and src_reg
are the same.
>
> >
> > Do you think it's feasible to explicitly initialize the register
> > values like it's done here:
> > https://github.com/google/kmsan/commit/813c0f3d45ebfa321d70b4b06cc054518dd1d90d
> > ?
> >
> > Thanks,
> > Alexander Potapenko
> > Software Engineer
> >
> > Google Germany GmbH
> > Erika-Mann-Straße, 33
> > 80636 München
> >
> > Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> > Registergericht und -nummer: Hamburg, HRB 86891
> > Sitz der Gesellschaft: Hamburg
> >



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg