Re: [PATCH v6 bpf-next 05/17] selftests/bpf: add selftests for new insn_array map

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/10/22 06:55AM, Eduard Zingerman wrote:
> On Wed, 2025-10-22 at 13:44 +0000, Anton Protopopov wrote:
> > On 25/10/21 04:51PM, Eduard Zingerman wrote:
> > > On Sun, 2025-10-19 at 20:21 +0000, Anton Protopopov wrote:
> > > > Add the following selftests for new insn_array map:
> > > > 
> > > >   * Incorrect instruction indexes are rejected
> > > >   * Two programs can't use the same map
> > > >   * BPF progs can't operate the map
> > > >   * no changes to code => map is the same
> > > >   * expected changes when instructions are added
> > > >   * expected changes when instructions are deleted
> > > >   * expected changes when multiple functions are present
> > > > 
> > > > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > > > ---
> > > 
> > > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > > 
> > > >  .../selftests/bpf/prog_tests/bpf_insn_array.c | 404 ++++++++++++++++++
> > > >  1 file changed, 404 insertions(+)
> > > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
> > > > 
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
> > > > new file mode 100644
> > > > index 000000000000..a4304ef5be13
> > > > --- /dev/null
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
> > > 
> > > [...]
> > > 
> > > > +static void check_bpf_no_lookup(void)
> > > 
> > > This one can be moved to prog_tests/bpf_insn_array.c, I think.
> > 
> > A typo? (This is a patch for the prog_tests/bpf_insn_array.c)
> 
> Yes, I mean progs/verifier_gotox.c, the one with inline assembly.

I think it should stay here. There will be other usages of the
instruction array, and neither should allow operations on it from
a BPF prog (indirect calls, static keys).

> > 
> > > > +{
> > > > +	struct bpf_insn insns[] = {
> > > > +		BPF_LD_MAP_FD(BPF_REG_1, 0),
> > > > +		BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> > > > +		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> > > > +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> > > > +		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> > > > +		BPF_EXIT_INSN(),
> > > > +	};
> > > > +	int prog_fd = -1, map_fd;
> > > > +
> > > > +	map_fd = map_create(BPF_MAP_TYPE_INSN_ARRAY, 1);
> > > > +	if (!ASSERT_GE(map_fd, 0, "map_create"))
> > > > +		return;
> > > > +
> > > > +	insns[0].imm = map_fd;
> > > > +
> > > > +	if (!ASSERT_EQ(bpf_map_freeze(map_fd), 0, "bpf_map_freeze"))
> > > > +		goto cleanup;
> > > > +
> > > > +	prog_fd = prog_load(insns, ARRAY_SIZE(insns), NULL, 0);
> > > > +	if (!ASSERT_EQ(prog_fd, -EINVAL, "program should have been rejected (prog_fd != -EINVAL)"))
> > > > +		goto cleanup;
> > > > +
> > > > +	/* correctness: check that prog is still loadable with normal map */
> > > > +	close(map_fd);
> > > > +	map_fd = map_create(BPF_MAP_TYPE_ARRAY, 1);
> > > > +	insns[0].imm = map_fd;
> > > > +	prog_fd = prog_load(insns, ARRAY_SIZE(insns), NULL, 0);
> > > > +	if (!ASSERT_GE(prog_fd, 0, "bpf(BPF_PROG_LOAD)"))
> > > > +		goto cleanup;
> > > > +
> > > > +cleanup:
> > > > +	close(prog_fd);
> > > > +	close(map_fd);
> > > > +}
> > > 
> > > [...]