Thread (19 messages) 19 messages, 7 authors, 2020-06-17

Re: [PATCH bpf-next 4/4] bpf: Add selftests for local_storage

From: KP Singh <hidden>
Date: 2020-06-16 15:54:43
Also in: bpf, linux-fsdevel, lkml

On 01-Jun 13:29, Andrii Nakryiko wrote:
On Tue, May 26, 2020 at 9:34 AM KP Singh [off-list ref] wrote:
quoted
From: KP Singh <redacted>

inode_local_storage:

* Hook to the file_open and inode_unlink LSM hooks.
* Create and unlink a temporary file.
* Store some information in the inode's bpf_local_storage during
  file_open.
* Verify that this information exists when the file is unlinked.

sk_local_storage:

* Hook to the socket_post_create and socket_bind LSM hooks.
* Open and bind a socket and set the sk_storage in the
  socket_post_create hook using the start_server helper.
* Verify if the information is set in the socket_bind hook.

Signed-off-by: KP Singh <redacted>
---
 .../bpf/prog_tests/test_local_storage.c       |  60 ++++++++
 .../selftests/bpf/progs/local_storage.c       | 139 ++++++++++++++++++
 2 files changed, 199 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_local_storage.c
 create mode 100644 tools/testing/selftests/bpf/progs/local_storage.c
[...]
quoted
+struct dummy_storage {
+       __u32 value;
+};
+
+struct {
+       __uint(type, BPF_MAP_TYPE_INODE_STORAGE);
+       __uint(map_flags, BPF_F_NO_PREALLOC);
+       __type(key, int);
+       __type(value, struct dummy_storage);
+} inode_storage_map SEC(".maps");
+
+struct {
+       __uint(type, BPF_MAP_TYPE_SK_STORAGE);
+       __uint(map_flags, BPF_F_NO_PREALLOC | BPF_F_CLONE);
+       __type(key, int);
+       __type(value, struct dummy_storage);
+} sk_storage_map SEC(".maps");
+
+/* Using vmlinux.h causes the generated BTF to be so big that the object
+ * load fails at btf__load.
+ */
That's first time I hear about such issue. Do you have an error log
from verifier?
Here's what I get when I do the following change.
--- a/tools/testing/selftests/bpf/progs/local_storage.c
+++ b/tools/testing/selftests/bpf/progs/local_storage.c
@@ -4,8 +4,8 @@
  * Copyright 2020 Google LLC.
  */
 
+#include "vmlinux.h"
 #include <errno.h>
-#include <linux/bpf.h>
 #include <stdbool.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
@@ -37,24 +37,6 @@ struct {
        __type(value, struct dummy_storage);
 } sk_storage_map SEC(".maps");
 
-/* Using vmlinux.h causes the generated BTF to be so big that the object
- * load fails at btf__load.
- */
-struct sock {} __attribute__((preserve_access_index));
-struct sockaddr {} __attribute__((preserve_access_index));
-struct socket {
-       struct sock *sk;
-} __attribute__((preserve_access_index));
-
-struct inode {} __attribute__((preserve_access_index));
-struct dentry {
-       struct inode *d_inode;
-} __attribute__((preserve_access_index));
-struct file {
-       struct inode *f_inode;
-} __attribute__((preserve_access_index));
./test_progs -t test_local_storage
libbpf: Error loading BTF: Invalid argument(22)
libbpf: magic: 0xeb9f
version: 1
flags: 0x0
hdr_len: 24
type_off: 0
type_len: 4488
str_off: 4488
str_len: 3012
btf_total_size: 7524

[1] STRUCT (anon) size=32 vlen=4
	type type_id=2 bits_offset=0
	map_flags type_id=6 bits_offset=64
	key type_id=8 bits_offset=128
	value type_id=9 bits_offset=192
[2] PTR (anon) type_id=4
[3] INT int size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[4] ARRAY (anon) type_id=3 index_type_id=5 nr_elems=28
[5] INT __ARRAY_SIZE_TYPE__ size=4 bits_offset=0 nr_bits=32 encoding=(none)
[6] PTR (anon) type_id=7
[7] ARRAY (anon) type_id=3 index_type_id=5 nr_elems=1
[8] PTR (anon) type_id=3
[9] PTR (anon) type_id=10
[10] STRUCT dummy_storage size=4 vlen=1
	value type_id=11 bits_offset=0
[11] TYPEDEF __u32 type_id=12

  [... More BTF Dump ...]

[91] TYPEDEF wait_queue_head_t type_id=175

  [... More BTF Dump ...]

[173] FWD super_block struct
[174] FWD vfsmount struct
[175] FWD wait_queue_head struct
[106] STRUCT socket_wq size=128 vlen=4
	wait type_id=91 bits_offset=0 Invalid member

libbpf: Error loading .BTF into kernel: -22.
libbpf: map 'inode_storage_map': failed to create: Invalid argument(-22)
libbpf: failed to load object 'local_storage'
libbpf: failed to load BPF skeleton 'local_storage': -22
test_test_local_storage:FAIL:skel_load lsm skeleton failed
#81 test_local_storage:FAIL

The failiure is in:

[106] STRUCT socket_wq size=128 vlen=4
	wait type_id=91 bits_offset=0 Invalid member
Clang is smart enough to trim down used types to only those that are
actually necessary, so too big BTF shouldn't be a thing. But let's try
to dig into this and fix whatever issue it is, before giving up :)
I was wrong about the size being an issue. The verifier thinks the BTF
is invalid and more specificially it thinks that the socket_wq's
member with type_id=91, i.e. typedef wait_queue_head_t is invalid. Am
I missing some toolchain patches?

- KP

quoted
+struct sock {} __attribute__((preserve_access_index));
+struct sockaddr {} __attribute__((preserve_access_index));
+struct socket {
+       struct sock *sk;
+} __attribute__((preserve_access_index));
+
+struct inode {} __attribute__((preserve_access_index));
+struct dentry {
+       struct inode *d_inode;
+} __attribute__((preserve_access_index));
+struct file {
+       struct inode *f_inode;
+} __attribute__((preserve_access_index));
+
+
[...]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help