Date:   Tue, 25 Jun 2019 21:42:05 +0200
From:   Krzesimir Nowak <krzesimir@...volk.io>
To:     netdev@...r.kernel.org
Cc:     Alban Crequy <alban@...volk.io>,
        Iago López Galeiras <iago@...volk.io>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        Krzesimir Nowak <krzesimir@...volk.io>
Subject: [bpf-next v2 00/10] Test the 32bit narrow reads

These patches try to test the fix made in commit e2f7fc0ac695 ("bpf:
fix undefined behavior in narrow load handling"). The problem existed
in the generated BPF bytecode that was doing a 32bit narrow read of a
64bit field, so to test it the code would need to be executed.
Currently the only such field exists in BPF_PROG_TYPE_PERF_EVENT,
which was not supported by bpf_prog_test_run().

I'm sending these patches to bpf-next now as they introduce a new
feature. But maybe some of those patches could go to the bpf branch?


There is a bit of yak shaving to do for the test to be run:

1. Print why the program could not be run (patch 1).

2. Some fixes for errno clobbering (patches 2 and 3).

3. Using bpf_prog_test_run_xattr, so I can pass ctx_in stuff too
   (patch 4).

4. Adding ctx stuff to struct bpf_test (patch 5).

5. Some tools headers syncing (patches 6 and 7).

6. Implement bpf_prog_test_run for perf event programs and test it
   (patches 8 and 9).


The last point is where I'm least sure how things should be done
properly:

1. There is a bunch of stuff to prepare for the
   bpf_perf_prog_read_value to work, and that stuff is very hacky. I
   would welcome some hints about how to set up the perf_event and
   perf_sample_data structs in a way that is a bit more future-proof
   than just setting some fields in a specific way, so some other code
   won't use some other fields (like setting event.oncpu to -1 to
   avoid event.pmu to be used for reading the value of the event).

2. The tests try to see if the test run for perf event sets up the
   context properly, so they verify the struct pt_regs contents. They
   way it is currently written Works For Me, but surely it won't work
   on other arch. So what would be the way forward? Just put the test
   case inside #ifdef __x86_64__?

3. Another thing in tests - I'm trying to make sure that the
   bpf_perf_prog_read_value helper works as it seems to be the only
   bpf perf helper that takes the ctx as a parameter. Is that enough
   or I should test other helpers too?


About the test itself - I'm not sure if it will work on a big endian
machine. I think it should, but I don't have anything handy here to
verify it.

Krzesimir Nowak (10):
  selftests/bpf: Print a message when tester could not run a program
  selftests/bpf: Avoid a clobbering of errno
  selftests/bpf: Avoid another case of errno clobbering
  selftests/bpf: Use bpf_prog_test_run_xattr
  selftests/bpf: Allow passing more information to BPF prog test run
  tools headers: Adopt compiletime_assert from kernel sources
  tools headers: sync struct bpf_perf_event_data
  bpf: Implement bpf_prog_test_run for perf event programs
  selftests/bpf: Add tests for bpf_prog_test_run for perf events progs
  selftests/bpf: Test correctness of narrow 32bit read on 64bit field

 kernel/trace/bpf_trace.c                      | 107 +++++++++++
 tools/include/linux/compiler.h                |  28 +++
 tools/include/uapi/linux/bpf_perf_event.h     |   1 +
 tools/testing/selftests/bpf/test_verifier.c   | 172 ++++++++++++++++--
 .../selftests/bpf/verifier/perf_event_run.c   |  93 ++++++++++
 .../bpf/verifier/perf_event_sample_period.c   |   8 +
 .../testing/selftests/bpf/verifier/var_off.c  |  20 ++
 7 files changed, 414 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/verifier/perf_event_run.c

-- 
2.20.1

Powered by blists - more mailing lists