bpf: Lift hashtab key_size limit

Currently key_size of hashtab is limited to MAX_BPF_STACK.
As the key of hashtab can also be a value from a per cpu map it can be
larger than MAX_BPF_STACK.

The use-case for this patch originates to implement allow/disallow
lists for files and file paths. The maximum length of file paths is
defined by PATH_MAX with 4096 chars including nul.
This limit exceeds MAX_BPF_STACK.

Changelog:

v5:
 - Fix cast overflow

v4:
 - Utilize BPF skeleton in tests
 - Rebase

v3:
 - Rebase

v2:
 - Add a test for bpf side

Signed-off-by: Florian Lehner <dev@der-flo.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20201029201442.596690-1-dev@der-flo.net

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 23f73d4649c9c9eaf6246f1088ddfee2d3875ea9..7bf18d92af410f1275c91e0f9bd94f7a5b30b5fc 100644 (file)
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -415,17 +415,11 @@ static int htab_map_alloc_check(union bpf_attr *attr)
            attr->value_size == 0)
                return -EINVAL;
 
-       if (attr->key_size > MAX_BPF_STACK)
-               /* eBPF programs initialize keys on stack, so they cannot be
-                * larger than max stack size
-                */
-               return -E2BIG;
-
-       if (attr->value_size >= KMALLOC_MAX_SIZE -
-           MAX_BPF_STACK - sizeof(struct htab_elem))
-               /* if value_size is bigger, the user space won't be able to
-                * access the elements via bpf syscall. This check also makes
-                * sure that the elem_size doesn't overflow and it's
+       if ((u64)attr->key_size + attr->value_size >= KMALLOC_MAX_SIZE -
+          sizeof(struct htab_elem))
+               /* if key_size + value_size is bigger, the user space won't be
+                * able to access the elements via bpf syscall. This check
+                * also makes sure that the elem_size doesn't overflow and it's
                 * kmalloc-able later in htab_map_update_elem()
                 */
                return -E2BIG;

diff --git a/tools/testing/selftests/bpf/prog_tests/hash_large_key.c b/tools/testing/selftests/bpf/prog_tests/hash_large_key.c
new file mode 100644 (file)
index 0000000..34684c0
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/hash_large_key.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "test_hash_large_key.skel.h"
+
+void test_hash_large_key(void)
+{
+       int err, value = 21, duration = 0, hash_map_fd;
+       struct test_hash_large_key *skel;
+
+       struct bigelement {
+               int a;
+               char b[4096];
+               long long c;
+       } key;
+       bzero(&key, sizeof(key));
+
+       skel = test_hash_large_key__open_and_load();
+       if (CHECK(!skel, "skel_open_and_load", "skeleton open/load failed\n"))
+               return;
+
+       hash_map_fd = bpf_map__fd(skel->maps.hash_map);
+       if (CHECK(hash_map_fd < 0, "bpf_map__fd", "failed\n"))
+               goto cleanup;
+
+       err = test_hash_large_key__attach(skel);
+       if (CHECK(err, "attach_raw_tp", "err %d\n", err))
+               goto cleanup;
+
+       err = bpf_map_update_elem(hash_map_fd, &key, &value, BPF_ANY);
+       if (CHECK(err, "bpf_map_update_elem", "errno=%d\n", errno))
+               goto cleanup;
+
+       key.c = 1;
+       err = bpf_map_lookup_elem(hash_map_fd, &key, &value);
+       if (CHECK(err, "bpf_map_lookup_elem", "errno=%d\n", errno))
+               goto cleanup;
+
+       CHECK_FAIL(value != 42);
+
+cleanup:
+       test_hash_large_key__destroy(skel);
+}

diff --git a/tools/testing/selftests/bpf/progs/test_hash_large_key.c b/tools/testing/selftests/bpf/progs/test_hash_large_key.c
new file mode 100644 (file)
index 0000000..473a227
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_hash_large_key.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct {
+       __uint(type, BPF_MAP_TYPE_HASH);
+       __uint(max_entries, 2);
+       __type(key, struct bigelement);
+       __type(value, __u32);
+} hash_map SEC(".maps");
+
+struct {
+       __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+       __uint(max_entries, 1);
+       __type(key, __u32);
+       __type(value, struct bigelement);
+} key_map SEC(".maps");
+
+struct bigelement {
+       int a;
+       char b[4096];
+       long long c;
+};
+
+SEC("raw_tracepoint/sys_enter")
+int bpf_hash_large_key_test(void *ctx)
+{
+       int zero = 0, err = 1, value = 42;
+       struct bigelement *key;
+
+       key = bpf_map_lookup_elem(&key_map, &zero);
+       if (!key)
+               return 0;
+
+       key->c = 1;
+       if (bpf_map_update_elem(&hash_map, key, &value, BPF_ANY))
+               return 0;
+
+       return 0;
+}
+

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 0d92ebcb335d1b7637641c07594c99cf3312b9fb..0ad3e6305ff03ca15d51ba659cae13a3d8fcac1a 100644 (file)
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1223,9 +1223,10 @@ out_map_in_map:
 
 static void test_map_large(void)
 {
+
        struct bigkey {
                int a;
-               char b[116];
+               char b[4096];
                long long c;
        } key;
        int fd, i, value;