From: KP Singh <kpsingh@chromium.org>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: KP Singh <kpsingh@chromium.org>,
open list <linux-kernel@vger.kernel.org>,
linux-fsdevel@vger.kernel.org, bpf <bpf@vger.kernel.org>,
linux-security-module@vger.kernel.org,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
James Morris <jmorris@namei.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Martin KaFai Lau <kafai@fb.com>,
Florent Revest <revest@chromium.org>
Subject: Re: [PATCH bpf-next 4/4] bpf: Add selftests for local_storage
Date: Tue, 16 Jun 2020 17:54:33 +0200 [thread overview]
Message-ID: <20200616155433.GA11971@google.com> (raw)
In-Reply-To: <CAEf4BzY0=Hh3O6qeD=2sMWpQRpHpizxH+nEA0hD0khPf3VAbhA@mail.gmail.com>
On 01-Jun 13:29, Andrii Nakryiko wrote:
> On Tue, May 26, 2020 at 9:34 AM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >
> > 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 <kpsingh@google.com>
> > ---
> > .../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
> >
>
> [...]
>
> > +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
> > +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));
> > +
> > +
>
> [...]
next prev parent reply other threads:[~2020-06-16 15:54 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 16:33 [PATCH bpf-next 0/4] Generalizing bpf_local_storage KP Singh
2020-05-26 16:33 ` [PATCH bpf-next 1/4] bpf: Generalize bpf_sk_storage KP Singh
2020-05-27 22:06 ` kbuild test robot
2020-05-26 16:33 ` [PATCH bpf-next 2/4] bpf: Implement bpf_local_storage for inodes KP Singh
2020-05-27 0:49 ` Alexei Starovoitov
2020-05-27 2:11 ` KP Singh
2020-05-27 5:08 ` Christoph Hellwig
2020-05-27 12:38 ` KP Singh
2020-05-27 16:41 ` Casey Schaufler
2020-05-27 17:09 ` KP Singh
2020-06-02 21:35 ` kbuild test robot
2020-05-26 16:33 ` [PATCH bpf-next 3/4] bpf: Allow local storage to be used from LSM programs KP Singh
2020-05-26 16:33 ` [PATCH bpf-next 4/4] bpf: Add selftests for local_storage KP Singh
2020-06-01 20:29 ` Andrii Nakryiko
2020-06-16 15:54 ` KP Singh [this message]
2020-06-16 19:25 ` Andrii Nakryiko
2020-06-16 20:40 ` Yonghong Song
2020-06-17 19:19 ` Yonghong Song
2020-06-17 19:26 ` KP Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200616155433.GA11971@google.com \
--to=kpsingh@chromium.org \
--cc=andrii.nakryiko@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jmorris@namei.org \
--cc=kafai@fb.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=revest@chromium.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).