[PATCH bpf v2 0/2] Reject writable map iterators on read-only maps

[PATCH bpf v2 0/2] Reject writable map iterators on read-only maps

[Adethya Srinivasan]

Reject writable map iterators on BPF_F_RDONLY_PROG maps and add an
attach-rejection selftest using the existing array-map iterator.

Testing on x86_64 under QEMU:
- Kernel and test_progs builds passed.
- Six selected bpf_iter subtests passed, including bpf_array_map_rdonly.

Changes in v2:
- Check BPF_F_RDONLY_PROG only; drop the frozen-map check.
- Shorten the commit message and correct the Fixes tags.
- Add a selftest expecting -EACCES when attaching dump_bpf_array_map()
  to a BPF_F_RDONLY_PROG array.

Adethya Srinivasan (2):
  bpf: Reject writable map iterators on read-only maps
  selftests/bpf: Check writable iterator attach to a read-only array

 kernel/bpf/map_iter.c                         | 12 +++++--
 .../selftests/bpf/prog_tests/bpf_iter.c       | 35 +++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

[PATCH bpf v2 1/2] bpf: Reject writable map iterators on read-only maps

[Adethya Srinivasan]

With CAP_BPF and CAP_PERFMON, an iterator program can write into a
frozen BPF_F_RDONLY_PROG array. The verifier treats the contents of
such an array as constants.

Reject writable map iterators on maps with BPF_F_RDONLY_PROG set.
Keep allowing read-only iterators.

Fixes: d3cc2ab546ad ("bpf: Implement bpf iterator for array maps")
Fixes: d6c4503cc296 ("bpf: Implement bpf iterator for hash maps")
Signed-off-by: Adethya Srinivasan <adethyasrinivasan@outlook.com>
---
 kernel/bpf/map_iter.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index a58c14869..7a769e5f5 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -99,8 +99,8 @@ static struct bpf_iter_reg bpf_map_reg_info = {
 
 /*
  * For an iterator that hands the live map value to the program as a writable
- * buffer: the verifier knows nothing of the map, so the special fields of the
- * value have to lie outside of what the program may write.
+ * buffer: the verifier knows nothing of the map, so read-only maps
+ * and special fields of the value have to be protected from writes.
  */
 int bpf_iter_map_check_value_write(const struct bpf_prog *prog,
 				   const struct bpf_map *map)
@@ -109,7 +109,13 @@ int bpf_iter_map_check_value_write(const struct bpf_prog *prog,
 	const struct btf_field *field;
 	u32 i;
 
-	if (IS_ERR_OR_NULL(rec) || !prog->aux->max_rdwr_write)
+	if (!prog->aux->max_rdwr_write)
+		return 0;
+
+	if (map->map_flags & BPF_F_RDONLY_PROG)
+		return -EACCES;
+
+	if (IS_ERR_OR_NULL(rec))
 		return 0;
 
 	for (i = 0; i < rec->cnt; i++) {

[PATCH bpf v2 2/2] selftests/bpf: Check writable iterator attach to a read-only array

[Adethya Srinivasan]

Reuse dump_bpf_array_map(), which writes to ctx->value, and attempt
to attach it to an array with BPF_F_RDONLY_PROG set. Assert that the
attach returns -EACCES and destroy the link if it unexpectedly succeeds.

The test fails on an unfixed kernel and passes with the fix applied.

Signed-off-by: Adethya Srinivasan <adethyasrinivasan@outlook.com>
---
 .../selftests/bpf/prog_tests/bpf_iter.c       | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index c69080ca1..5249b07cd 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -1100,6 +1100,39 @@ static void test_bpf_array_map(void)
 	bpf_iter_bpf_array_map__destroy(skel);
 }
 
+static void test_bpf_array_map_rdonly(void)
+{
+	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
+	struct bpf_iter_bpf_array_map *skel;
+	union bpf_iter_link_info linfo = {};
+	struct bpf_link *link;
+	int err;
+
+	skel = bpf_iter_bpf_array_map__open();
+	if (!ASSERT_OK_PTR(skel, "bpf_iter_bpf_array_map__open"))
+		return;
+
+	err = bpf_map__set_map_flags(skel->maps.arraymap1, BPF_F_RDONLY_PROG);
+	if (!ASSERT_OK(err, "set_map_flags"))
+		goto out;
+
+	err = bpf_iter_bpf_array_map__load(skel);
+	if (!ASSERT_OK(err, "bpf_iter_bpf_array_map__load"))
+		goto out;
+
+	linfo.map.map_fd = bpf_map__fd(skel->maps.arraymap1);
+	opts.link_info = &linfo;
+	opts.link_info_len = sizeof(linfo);
+	link = bpf_program__attach_iter(skel->progs.dump_bpf_array_map, &opts);
+	err = libbpf_get_error(link);
+	ASSERT_EQ(err, -EACCES, "attach_rdonly_write_iter");
+	if (!err)
+		bpf_link__destroy(link);
+
+out:
+	bpf_iter_bpf_array_map__destroy(skel);
+}
+
 static void test_bpf_array_map_iter_fd(void)
 {
 	struct bpf_iter_bpf_array_map *skel;
@@ -1770,6 +1803,8 @@ void test_bpf_iter(void)
 		test_bpf_percpu_hash_map();
 	if (test__start_subtest("bpf_array_map"))
 		test_bpf_array_map();
+	if (test__start_subtest("bpf_array_map_rdonly"))
+		test_bpf_array_map_rdonly();
 	if (test__start_subtest("bpf_array_map_iter_fd"))
 		test_bpf_array_map_iter_fd();
 	if (test__start_subtest("bpf_percpu_array_map"))