netdev - Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390

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]

Message-ID: <20190711203508.GC16709@mini-arch>
Date:   Thu, 11 Jul 2019 13:35:08 -0700
From:   Stanislav Fomichev <sdf@...ichev.me>
To:     Ilya Leoshkevich <iii@...ux.ibm.com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ys114321@...il.com,
        daniel@...earbox.net, davem@...emloft.net, ast@...nel.org
Subject: Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling
 loop{1,2,3}.c on s390

On 07/11, Ilya Leoshkevich wrote:
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
> 
> This patch series consists of three preparatory commits, which make it
> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
> 
> > > Will this also work for 32-bit x86?
> > Thanks, this is a good catch: this builds, but makes 64-bit accesses, as
> > if it used the 64-bit variant of pt_regs. I will fix this.
> I found four problems in this area:
> 
> 1. Selftest tracing progs are built with -target bpf, leading to struct
>    pt_regs and friends being interpreted incorrectly.
> 2. When the Makefile is adjusted to build them without -target bpf, it
>    still lacks -m32/-m64, leading to a similar issue.
> 3. There is no __i386__ define, leading to incorrect userspace struct
>    pt_regs variant being chosen for x86.
> 4. Finally, there is an issue in my patch: when 1-3 are fixed, it fails
>    to build, since i386 defines yet another set of field names.
> 
> I will send fixes for problems 1-3 separately, I believe for this patch
> series to be correct, it's enough to fix #4 (which I did by adding
> another #ifdef).
> 
> I've also changed ARCH to SRCARCH in patch #1, since while ARCH can be
> e.g. "i386", SRCARCH always corresponds to directory names under arch/.
> 
> v1->v2: Split into multiple patches.
> v2->v3: Added arm64 support.
> v3->v4: Added i386 support, use SRCARCH instead of ARCH.
Still looks good to me, thanks!

Reviewed-by: Stanislav Fomichev <sdf@...gle.com>

Again, should probably go via bpf to fix the existing tests, not bpf-next
(but I see bpf tree is not synced with net tree yet).

> Signed-off-by: Ilya Leoshkevich <iii@...ux.ibm.com>
> 
>