bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Delyan Kratunov <delyank@fb.com>
To: "daniel@iogearbox.net" <daniel@iogearbox.net>,
	"ast@kernel.org" <ast@kernel.org>,
	"andrii@kernel.org" <andrii@kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>
Subject: [PATCH bpf-next v2 0/1] Avoid size mismatches in skeletons
Date: Tue, 15 Feb 2022 00:26:41 +0000	[thread overview]
Message-ID: <cover.1644884357.git.delyank@fb.com> (raw)

As reported in [0], kernel and userspace can sometimes disagree
on the size of a type. This leads to trouble when userspace maps the memory of
a bpf program and reads/writes to it assuming a different memory layout.

With this change, the skeletons now contain size asserts to ensure the
types in userspace are compatible in size with the types in the bpf program.
In particular, we emit asserts for all top-level fields in the data/rodata/bss/etc
structs, but not recursively for the individual members inside - this strikes a
compromise between diagnostics precision and still catching all possible size
mismatches.

The generated asserts are somewhat ugly but are able to handle anonymous structs:

  struct test_skeleton__data {
          int in1;
          char __pad0[4];
          long long in2;
          int out1;
          char __pad1[4];
          long long out2;
  } *data;
  BPF_STATIC_ASSERT(sizeof(((struct test_skeleton__data*)0)->in1) == 4, "unexpe
cted size of field in1");
  BPF_STATIC_ASSERT(sizeof(((struct test_skeleton__data*)0)->in2) == 8, "unexpe
cted size of field in2");
  BPF_STATIC_ASSERT(sizeof(((struct test_skeleton__data*)0)->out1) == 4, "unexp
ected size of field out1");
  BPF_STATIC_ASSERT(sizeof(((struct test_skeleton__data*)0)->out2) == 8, "unexp
ected size of field out2");
  struct test_skeleton__rodata {
          struct {
                  int in6;
          } in;
  } *rodata;
  BPF_STATIC_ASSERT(sizeof(((struct test_skeleton__rodata*)0)->in) == 4, "unexp
ected size of field in");

I'm open to pushing more of the ugliness into a macro, I was going primarily for
simplicity in the diagnostic messages (it's unfortunate enough that we need a level
of macro expansion for C++ support). If we need this to be prettier, what's a good
header I could push any extra complexity into, so it's not spelled out in gen.c?

Delyan Kratunov (1):
  bpftool: bpf skeletons assert type sizes

 tools/bpf/bpftool/gen.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

--
2.34.1

             reply	other threads:[~2022-02-15  0:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-15  0:26 Delyan Kratunov [this message]
2022-02-15  0:26 ` [PATCH bpf-next v2 1/1] bpftool: bpf skeletons assert type sizes Delyan Kratunov
2022-02-15  5:11   ` Andrii Nakryiko
2022-02-15 17:27     ` Delyan Kratunov
2022-02-15 17:55       ` Andrii Nakryiko

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=cover.1644884357.git.delyank@fb.com \
    --to=delyank@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    /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).