On Tue, Nov 20, 2018 at 03:43:05PM +0000, Lorenz Bauer wrote: > Require size_out to be non-NULL if data_out is given. This prevents > accidental overwriting of process memory after the output buffer. > > Adjust callers of bpf_prog_test_run to this behaviour. > > Signed-off-by: Lorenz Bauer <lmb@xxxxxxxxxxxxxx> > --- > tools/lib/bpf/bpf.c | 7 ++++++- > tools/testing/selftests/bpf/test_progs.c | 10 ++++++++++ > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index 961e1b9fc592..1a835ff27486 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -407,15 +407,20 @@ int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size, > union bpf_attr attr; > int ret; > > + if (data_out && !size_out) > + return -EINVAL; > + > bzero(&attr, sizeof(attr)); > attr.test.prog_fd = prog_fd; > attr.test.data_in = ptr_to_u64(data); > attr.test.data_out = ptr_to_u64(data_out); > attr.test.data_size_in = size; > + if (data_out) > + attr.test.data_size_out = *size_out; > attr.test.repeat = repeat; > > ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr)); > - if (size_out) > + if (data_out) > *size_out = attr.test.data_size_out; > if (retval) > *retval = attr.test.retval; > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index c1e688f61061..299938603cb6 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -150,6 +150,7 @@ static void test_xdp(void) > bpf_map_update_elem(map_fd, &key4, &value4, 0); > bpf_map_update_elem(map_fd, &key6, &value6, 0); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4), > buf, &size, &retval, &duration); > > @@ -158,6 +159,7 @@ static void test_xdp(void) > "err %d errno %d retval %d size %d\n", > err, errno, retval, size); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, &pkt_v6, sizeof(pkt_v6), > buf, &size, &retval, &duration); This will surely break existing bpf_prog_test_run users. Like it will break our testing framework. we can fix out stuff and libbpf is a user space library, but I don't think that this is the case to invoke such pain. libbpf's bpf_prog_test_run() should be a simple wrapper on top of syscall. I don't think it should be making such restrictions on api. btw patch 1 looks good to me.
