On Thu, Oct 20, 2022 at 9:07 AM Dave Marchevsky <davemarchevsky@xxxxxx> wrote:
>
> Modify iter prog in existing bpf_iter_bpf_array_map.c, which currently
> dumps arraymap key/val, to also do a write of (val, key) into a
> newly-added hashmap. Confirm that the write succeeds as expected by
> modifying the userspace runner program.
>
> Before a change added in an earlier commit - considering PTR_TO_BUF reg
> a valid input to helpers which expect MAP_{KEY,VAL} - the verifier
> would've rejected this prog change due to type mismatch. Since using
> current iter's key/val to access a separate map is a reasonable usecase,
> let's add support for it.
>
> Note that the test prog cannot directly write (val, key) into hashmap
> via bpf_map_update_elem when both come from iter context because key is
> marked MEM_RDONLY. This is due to bpf_map_update_elem - and other basic
> map helpers - taking ARG_PTR_TO_MAP_{KEY,VALUE} w/o MEM_RDONLY type
> flag. bpf_map_{lookup,update,delete}_elem don't modify their
> input key/val so it should be possible to tag their args READONLY, but
> due to the ubiquitous use of these helpers and verifier checks for
> type == MAP_VALUE, such a change is nontrivial and seems better to
> address in a followup series.
Agree about addressing it separately, but I'm not sure what's
non-trivial or dangerous? If I remember correctly, MEM_RDONLY on
helper input arg just means that it accepts both read-only and
read-write views. While the input argument doesn't have MEM_RDONLY we
accept *only* read/write memory views. So basically adding MEM_RDONLY
in BPF helper proto makes it more general and permissive in what can
be passed into it. I think that's a good change, we added tons of
MEM_RDONLY to helpers that were accepting PTR_TO_MEM already.
But anyways, patch looks good:
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> Also fixup some 'goto's in test runner's map checking loop.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> ---
> .../selftests/bpf/prog_tests/bpf_iter.c | 20 ++++++++++++------
> .../bpf/progs/bpf_iter_bpf_array_map.c | 21 ++++++++++++++++++-
> 2 files changed, 34 insertions(+), 7 deletions(-)
>
[...]
