selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs

The current assumption is that the lifetime of a cgroup storage
is tied to the program's attachment. The storage is created in
cgroup_bpf_attach, and released upon cgroup_bpf_detach and
cgroup_bpf_release.

Because the current semantics is that each attachment gets a
completely independent cgroup storage, and you can have multiple
programs attached to the same (cgroup, attach type) pair, the key
of the CGROUP_STORAGE map, looking up the map with this pair could
yield multiple storages, and that is not permitted. Therefore,
the kernel verifier checks that two programs cannot share the same
CGROUP_STORAGE map, even if they have different expected attach
types, considering that the actual attach type does not always
have to be equal to the expected attach type.

The test creates a CGROUP_STORAGE map and make it shared across
two different programs, one cgroup_skb/egress and one /ingress.
It asserts that the two programs cannot be both loaded, due to
verifier failure from the above reason.

Signed-off-by: YiFei Zhu <zhuyifei@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/30a6b0da67ae6b0296c4d511bfb19c5f3d035916.1595565795.git.zhuyifei@google.com

diff --git a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
index e90e0547d759c39ba29c4eb93eb2b1dc6871fcbe..1c765342369864e334a1271d0205e67de753e53a 100644 (file)
--- a/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cg_storage_multi.c
@@ -8,7 +8,10 @@
 #include <cgroup_helpers.h>
 #include <network_helpers.h>
 
+#include "progs/cg_storage_multi.h"
+
 #include "cg_storage_multi_egress_only.skel.h"
+#include "cg_storage_multi_egress_ingress.skel.h"
 
 #define PARENT_CGROUP "/cgroup_storage"
 #define CHILD_CGROUP "/cgroup_storage/child"
@@ -16,10 +19,10 @@
 static int duration;
 
 static bool assert_storage(struct bpf_map *map, const char *cgroup_path,
-                          __u32 expected)
+                          struct cgroup_value *expected)
 {
        struct bpf_cgroup_storage_key key = {0};
-       __u32 value;
+       struct cgroup_value value;
        int map_fd;
 
        map_fd = bpf_map__fd(map);
@@ -29,8 +32,8 @@ static bool assert_storage(struct bpf_map *map, const char *cgroup_path,
        if (CHECK(bpf_map_lookup_elem(map_fd, &key, &value) < 0,
                  "map-lookup", "errno %d", errno))
                return true;
-       if (CHECK(value != expected,
-                 "assert-storage", "got %u expected %u", value, expected))
+       if (CHECK(memcmp(&value, expected, sizeof(struct cgroup_value)),
+                 "assert-storage", "storages differ"))
                return true;
 
        return false;
@@ -39,7 +42,7 @@ static bool assert_storage(struct bpf_map *map, const char *cgroup_path,
 static bool assert_storage_noexist(struct bpf_map *map, const char *cgroup_path)
 {
        struct bpf_cgroup_storage_key key = {0};
-       __u32 value;
+       struct cgroup_value value;
        int map_fd;
 
        map_fd = bpf_map__fd(map);
@@ -86,6 +89,7 @@ out_clean:
 static void test_egress_only(int parent_cgroup_fd, int child_cgroup_fd)
 {
        struct cg_storage_multi_egress_only *obj;
+       struct cgroup_value expected_cgroup_value;
        struct bpf_link *parent_link = NULL, *child_link = NULL;
        bool err;
 
@@ -109,7 +113,9 @@ static void test_egress_only(int parent_cgroup_fd, int child_cgroup_fd)
        if (CHECK(obj->bss->invocations != 1,
                  "first-invoke", "invocations=%d", obj->bss->invocations))
                goto close_bpf_object;
-       if (assert_storage(obj->maps.cgroup_storage, PARENT_CGROUP, 1))
+       expected_cgroup_value = (struct cgroup_value) { .egress_pkts = 1 };
+       if (assert_storage(obj->maps.cgroup_storage,
+                          PARENT_CGROUP, &expected_cgroup_value))
                goto close_bpf_object;
        if (assert_storage_noexist(obj->maps.cgroup_storage, CHILD_CGROUP))
                goto close_bpf_object;
@@ -129,9 +135,13 @@ static void test_egress_only(int parent_cgroup_fd, int child_cgroup_fd)
        if (CHECK(obj->bss->invocations != 3,
                  "second-invoke", "invocations=%d", obj->bss->invocations))
                goto close_bpf_object;
-       if (assert_storage(obj->maps.cgroup_storage, PARENT_CGROUP, 2))
+       expected_cgroup_value = (struct cgroup_value) { .egress_pkts = 2 };
+       if (assert_storage(obj->maps.cgroup_storage,
+                          PARENT_CGROUP, &expected_cgroup_value))
                goto close_bpf_object;
-       if (assert_storage(obj->maps.cgroup_storage, CHILD_CGROUP, 1))
+       expected_cgroup_value = (struct cgroup_value) { .egress_pkts = 1 };
+       if (assert_storage(obj->maps.cgroup_storage,
+                          CHILD_CGROUP, &expected_cgroup_value))
                goto close_bpf_object;
 
 close_bpf_object:
@@ -141,6 +151,20 @@ close_bpf_object:
        cg_storage_multi_egress_only__destroy(obj);
 }
 
+static void test_egress_ingress(int parent_cgroup_fd, int child_cgroup_fd)
+{
+       struct cg_storage_multi_egress_ingress *obj;
+
+       /* Cannot load both programs due to verifier failure:
+        * "only one cgroup storage of each type is allowed"
+        */
+       obj = cg_storage_multi_egress_ingress__open_and_load();
+       CHECK(obj || errno != EBUSY,
+             "skel-load", "errno %d, expected EBUSY", errno);
+
+       cg_storage_multi_egress_ingress__destroy(obj);
+}
+
 void test_cg_storage_multi(void)
 {
        int parent_cgroup_fd = -1, child_cgroup_fd = -1;
@@ -155,6 +179,9 @@ void test_cg_storage_multi(void)
        if (test__start_subtest("egress_only"))
                test_egress_only(parent_cgroup_fd, child_cgroup_fd);
 
+       if (test__start_subtest("egress_ingress"))
+               test_egress_ingress(parent_cgroup_fd, child_cgroup_fd);
+
 close_cgroup_fd:
        close(child_cgroup_fd);
        close(parent_cgroup_fd);

diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h
new file mode 100644 (file)
index 0000000..a0778fe
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __PROGS_CG_STORAGE_MULTI_H
+#define __PROGS_CG_STORAGE_MULTI_H
+
+#include <asm/types.h>
+
+struct cgroup_value {
+       __u32 egress_pkts;
+       __u32 ingress_pkts;
+};
+
+#endif

diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_ingress.c b/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_ingress.c
new file mode 100644 (file)
index 0000000..9ce3868
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_ingress.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright 2020 Google LLC.
+ */
+
+#include <errno.h>
+#include <linux/bpf.h>
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <bpf/bpf_helpers.h>
+
+#include "progs/cg_storage_multi.h"
+
+struct {
+       __uint(type, BPF_MAP_TYPE_CGROUP_STORAGE);
+       __type(key, struct bpf_cgroup_storage_key);
+       __type(value, struct cgroup_value);
+} cgroup_storage SEC(".maps");
+
+__u32 invocations = 0;
+
+SEC("cgroup_skb/egress")
+int egress(struct __sk_buff *skb)
+{
+       struct cgroup_value *ptr_cg_storage =
+               bpf_get_local_storage(&cgroup_storage, 0);
+
+       __sync_fetch_and_add(&ptr_cg_storage->egress_pkts, 1);
+       __sync_fetch_and_add(&invocations, 1);
+
+       return 1;
+}
+
+SEC("cgroup_skb/ingress")
+int ingress(struct __sk_buff *skb)
+{
+       struct cgroup_value *ptr_cg_storage =
+               bpf_get_local_storage(&cgroup_storage, 0);
+
+       __sync_fetch_and_add(&ptr_cg_storage->ingress_pkts, 1);
+       __sync_fetch_and_add(&invocations, 1);
+
+       return 1;
+}

diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_only.c b/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_only.c
index ec0165d07105581be013a59f075fd3b422bf3bd1..44ad46b3353960fd60a43e170f044790ea8fe803 100644 (file)
--- a/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_only.c
+++ b/tools/testing/selftests/bpf/progs/cg_storage_multi_egress_only.c
@@ -10,10 +10,12 @@
 #include <linux/udp.h>
 #include <bpf/bpf_helpers.h>
 
+#include "progs/cg_storage_multi.h"
+
 struct {
        __uint(type, BPF_MAP_TYPE_CGROUP_STORAGE);
        __type(key, struct bpf_cgroup_storage_key);
-       __type(value, __u32);
+       __type(value, struct cgroup_value);
 } cgroup_storage SEC(".maps");
 
 __u32 invocations = 0;
@@ -21,9 +23,10 @@ __u32 invocations = 0;
 SEC("cgroup_skb/egress")
 int egress(struct __sk_buff *skb)
 {
-       __u32 *ptr_cg_storage = bpf_get_local_storage(&cgroup_storage, 0);
+       struct cgroup_value *ptr_cg_storage =
+               bpf_get_local_storage(&cgroup_storage, 0);
 
-       __sync_fetch_and_add(ptr_cg_storage, 1);
+       __sync_fetch_and_add(&ptr_cg_storage->egress_pkts, 1);
        __sync_fetch_and_add(&invocations, 1);
 
        return 1;