netdev - Re: [RFC PATCH v3 bpf-next 2/5] bpf/verifier: rewrite subprog boundary detection

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

Message-ID: <2a368af7-4b2c-545f-0e85-826c93d4f58b@solarflare.com>
Date:   Tue, 1 May 2018 21:40:16 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Daniel Borkmann <daniel@...earbox.net>, <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH v3 bpf-next 2/5] bpf/verifier: rewrite subprog
 boundary detection

On 18/04/18 00:48, Alexei Starovoitov wrote:
> as I was saying before this is no go.
> subprogno is meaningless in the hierarchy of: prog -> func -> bb -> insn
> Soon bpf will have libraries and this field would need to become
> a pointer back to bb or func structure creating unnecessary circular dependency.
I'm afraid I don't follow.  Why can't func numbers (and later, bb numbers)
 be per-prog?  When verifier is linking multiple progs together it will
 necessarily have the subprog-info for each prog, and when making cross-
 prog calls it'll have to already know which prog it's calling into; I
 don't see any reason why the index into a prog's subprog_info array
 should become "meaningless" in such a setup.
Besides, subprogno is how the rest of the verifier currently identifies a
 func, and in the absence of any indication of how anything different
 will be implemented, that's what an incremental patch has to work with.

If you're worried about the SPOT violation from having both
 aux->subprogno and subprog_info->start... well, we could actually get
 rid of the latter!  Uses of it are:
* carving up insn arrays in jit_subprogs().  Could be done based on
  aux->subprogno instead (v1 of this series did that)
* checking CALL destination is at start of function.  That could be done
  by putting a flag in the aux_data to mark "this insn is at the start of
  its subprog".  Doesn't even need to increase memory usage: it could be
  done by ORing a flag (0x8000, say) into aux->subprogno; or we could
  replace 'bool seen;' with 'u8 flags;' again with no extra memory used.
* a few verbose() messages.
That would have another nice consequence, in that adjust_subprog_starts()
 could go away - another code simplification resulting from use of the
 right data structures.

Btw, sorry for delay in responding; got bogged down in some sfc driver
 work.

-Ed