On Mon, 24 Sep 2018 at 05:22, Daniel Borkmann <dan...@iogearbox.net> wrote: > > On 09/21/2018 07:10 PM, Joe Stringer wrote: > > reference tracking: leak potential reference > > reference tracking: leak potential reference on stack > > reference tracking: leak potential reference on stack 2 > > reference tracking: zero potential reference > > reference tracking: copy and zero potential references > > reference tracking: release reference without check > > reference tracking: release reference > > reference tracking: release reference twice > > reference tracking: release reference twice inside branch > > reference tracking: alloc, check, free in one subbranch > > reference tracking: alloc, check, free in both subbranches > > reference tracking in call: free reference in subprog > > reference tracking in call: free reference in subprog and outside > > reference tracking in call: alloc & leak reference in subprog > > reference tracking in call: alloc in subprog, release outside > > reference tracking in call: sk_ptr leak into caller stack > > reference tracking in call: sk_ptr spill into caller stack > > > > Signed-off-by: Joe Stringer <j...@wand.net.nz> > > Acked-by: Alexei Starovoitov <a...@kernel.org> > > --- > > tools/testing/selftests/bpf/test_verifier.c | 359 ++++++++++++++++++++ > > 1 file changed, 359 insertions(+) > > I think this here needs to have some more test cases that we current do not > track but > should in order to have better coverage. At minimum what comes to mind > additionally: > > - verifier interaction with LD_ABS, LD_IND > - verifier interaction with tail calls (e.g. try to leak socket, > socket_or_null, etc, > but should also have a positive test where we drop ref before tail call to > show it > works in combination) > - Try to mangle a socket and socket_or_null pointer with ALU ops and pass it > to helper > - Try to access the socket data fields after we released its reference > - Access socket member fields in general (I think not present right now) > - Use direct packet access in combination with lookup helper (it's enabled > via pkt_access = true in the helper, so we should also test for it here to > make > sure future changes don't break it)
Great suggestions, I think that the LD_ABS/LD_IND ones are good candidates for the assembly-level tests here. For the remainder, I plan to add them to the C cases. That'll be easier to review and easier to understand if they ever get broken. Regarding the direct packet access with lookup, that's already in this series in the C verifier tests patch.
