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

Edward Cree
2018-05-01T20:40:22Z

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