netdev - Re: csum_partial() on different archs (selftest/bpf)

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAJ+HfNiE5Oa25QgdAdKzfk-=X45hXLKk_t+ZCiSaeFVTzgzsrw@mail.gmail.com>
Date:   Fri, 13 Nov 2020 14:22:16 +0100
From:   Björn Töpel <bjorn.topel@...il.com>
To:     Jean-Philippe Brucker <jean-philippe@...aro.org>,
        viro@...iv.linux.org.uk
Cc:     Netdev <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Tom Herbert <tom@...bertland.com>,
        Anders Roxell <anders.roxell@...il.com>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: csum_partial() on different archs (selftest/bpf)

On Fri, 13 Nov 2020 at 13:25, Jean-Philippe Brucker
<jean-philippe@...aro.org> wrote:
>
> Hi,
>
> On Fri, Nov 13, 2020 at 11:36:08AM +0100, Björn Töpel wrote:
> > I was running the selftest/bpf on riscv, and had a closer look at one
> > of the failing cases:
> >
> >   #14/p valid read map access into a read-only array 2 FAIL retval
> > 65507 != -29 (run 1/1)
> >
> > The test does a csum_partial() call via a BPF helper. riscv uses the
> > generic implementation. arm64 uses the generic csum_partial() and fail
> > in the same way [1].
>
> It's worse than that, because arm64, parisc, alpha and others implement
> do_csum(), called by the generic csum_partial(), and those all return a
> 16-bit value.
>
> It would be good to firstly formalize the size of the value returned by
> the bpf_csum_diff() helper, because it's not clear from the doc (and the
> helper returns a s64).
>
> Then homogenizing the csum_partial() implementations is difficult. One way
> forward, without having to immediately rewrite all arch-specific
> implementations, would be to replace csum_partial() and do_csum() with
> csum_partial_32(), csum_partial_16(), do_csum_32() and do_csum_16(). That
> way we can use a generic implementation of the 32-bit variant if the
> arch-specific implementation is 16-bit.
>

Folding Al's input to this reply.

I think the bpf_csum_diff() is supposed to be used in combination with
another helper(s) (e.g. bpf_l4_csum_replace) so I'd guess the returned
__wsum should be seen as an opaque value, not something BPF userland
can rely on.

So, for this specific test case, it's probably best to just update the
test case (as Eric suggested).


Cheers, and thanks for the input!
Björn