On Mon, Jul 13, 2020 at 10:48 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote:
>
> Exercise verifier access checks for bpf_sk_lookup context fields.
>
> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> ---
>
LGTM.
Acked-by: Andrii Nakryiko <andriin@xxxxxx>
> Notes:
> v4:
> - Bring back tests for narrow loads.
>
> v3:
> - Consolidate ACCEPT tests into one.
> - Deduplicate REJECT tests and arrange them into logical groups.
> - Add tests for out-of-bounds and unaligned access.
> - Cover access to newly introduced 'sk' field.
>
> v2:
> - Adjust for fields renames in struct bpf_sk_lookup.
>
> .../selftests/bpf/verifier/ctx_sk_lookup.c | 471 ++++++++++++++++++
> 1 file changed, 471 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/verifier/ctx_sk_lookup.c
>
[...]
> + /* 1-byte read from local_port field */
> + BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port)),
> + BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port) + 1),
> + BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port) + 2),
> + BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port) + 3),
> + /* 2-byte read from local_port field */
> + BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port)),
> + BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port) + 2),
> + /* 4-byte read from local_port field */
> + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, local_port)),
> +
> + /* 8-byte read from sk field */
> + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, sk)),
> + BPF_EXIT_INSN(),
> + },
> + .result = ACCEPT,
> + .prog_type = BPF_PROG_TYPE_SK_LOOKUP,
> + .expected_attach_type = BPF_SK_LOOKUP,
> +},
This looks like a common class of tests which can be auto-generated
just from the list of fields and their sizes. Something for someone's
wishlist, though.
> +/* invalid 8-byte reads from a 4-byte fields in bpf_sk_lookup */
> +{
> + "invalid 8-byte read from bpf_sk_lookup family field",
> + .insns = {
> + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
> + offsetof(struct bpf_sk_lookup, family)),
> + BPF_EXIT_INSN(),
> + },
> + .errstr = "invalid bpf_context access",
> + .result = REJECT,
> + .prog_type = BPF_PROG_TYPE_SK_LOOKUP,
> + .expected_attach_type = BPF_SK_LOOKUP,
> +},
[...]
