Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: KP Singh <kpsingh@chromium.org>
Cc: "open list" <linux-kernel@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>,
	"Kees Cook" <keescook@chromium.org>,
	"Thomas Garnier" <thgarnie@chromium.org>,
	"Michael Halcrow" <mhalcrow@google.com>,
	"Paul Turner" <pjt@google.com>,
	"Brendan Gregg" <brendan.d.gregg@gmail.com>,
	"Jann Horn" <jannh@google.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Christian Brauner" <christian@brauner.io>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Florent Revest" <revest@chromium.org>,
	"Brendan Jackman" <jackmanb@chromium.org>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Mauro Carvalho Chehab" <mchehab+samsung@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Stanislav Fomichev" <sdf@google.com>,
	"Quentin Monnet" <quentin.monnet@netronome.com>,
	"Andrey Ignatov" <rdna@fb.com>, "Joe Stringer" <joe@wand.net.nz>
Subject: Re: [PATCH bpf-next v2 08/10] tools/libbpf: Add support for BPF_PROG_TYPE_LSM
Date: Thu, 16 Jan 2020 11:10:57 -0800
Message-ID: <CAEf4BzYa+m0OCZXqnNsab+q3+HLboL45eRdBrGdZD+6Z4CRSiQ@mail.gmail.com> (raw)
In-Reply-To: <20200116124955.GD240584@google.com>

On Thu, Jan 16, 2020 at 4:49 AM KP Singh <kpsingh@chromium.org> wrote:
>
> Thanks for the review Andrii!
>
> I will incorporate the fixes in the next revision.
>
> On 15-Jan 13:19, Andrii Nakryiko wrote:
> > On Wed, Jan 15, 2020 at 9:13 AM KP Singh <kpsingh@chromium.org> wrote:
> > >
> > > From: KP Singh <kpsingh@google.com>
> > >
> > > * Add functionality in libbpf to attach eBPF program to LSM hooks
> > > * Lookup the index of the LSM hook in security_hook_heads and pass it in
> > >   attr->lsm_hook_index
> > >
> > > Signed-off-by: KP Singh <kpsingh@google.com>
> > > ---
> > >  tools/lib/bpf/bpf.c      |   6 +-
> > >  tools/lib/bpf/bpf.h      |   1 +
> > >  tools/lib/bpf/libbpf.c   | 143 ++++++++++++++++++++++++++++++++++-----
> > >  tools/lib/bpf/libbpf.h   |   4 ++
> > >  tools/lib/bpf/libbpf.map |   3 +
> > >  5 files changed, 138 insertions(+), 19 deletions(-)
> > >

[...]

> >
> > > +{
> > > +       struct btf *btf = bpf_find_kernel_btf();
> >
> > ok, it's probably time to do this right. Let's ensure we load kernel
> > BTF just once, keep it inside bpf_object while we need it and then
> > release it after successful load. We are at the point where all the
> > new types of program is loading/releasing kernel BTF for every section
> > and it starts to feel very wasteful.
>
> Sure, will give it a shot in v3.

thanks!

[...]

> >
> > > +               if (!strcmp(btf__name_by_offset(btf, m->name_off), name))
> > > +                       return j + 1;
> >
> > I looked briefly through kernel-side patch introducing lsm_hook_index,
> > but it didn't seem to explain why this index needs to be (unnaturally)
> > 1-based. So asking here first as I'm looking through libbpf changes?
>
> The lsm_hook_idx is one-based as it makes it easy to validate the
> input. If we make it zero-based it's hard to check if the user
> intended to attach to the LSM hook at index 0 or did not set it.

Think about providing FDs. 0 is a valid, though rarely
intended/correct value. Yet we don't make all FD arguments
artificially 1-based, right? This extra +1/-1 translation just makes
for more confusing interface, IMO. If user "accidentally" guessed type
signature of very first hook, well, so be it... If not, BPF verifier
will politely refuse. Seems like enough protection.

>
> We are then up to the verifier to reject the loaded program which
> may or may not match the signature of the hook at lsm_hook_idx = 0.
>
> I will clarify this in the commit log as well.
>

[...]

  parent reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 17:13 [PATCH bpf-next v2 00/10] MAC and Audit policy using eBPF (KRSI) KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 01/10] bpf: btf: Make some of the API visible outside BTF KP Singh
2020-01-18 12:44   ` kbuild test robot
2020-01-20 11:00     ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 02/10] bpf: lsm: Add a skeleton and config options KP Singh
2020-01-16  7:04   ` Casey Schaufler
2020-01-16 12:52     ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 03/10] bpf: lsm: Introduce types for eBPF based LSM KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM KP Singh
2020-01-15 17:30   ` Stephen Smalley
2020-01-16  9:48     ` KP Singh
2020-01-16  6:33   ` Casey Schaufler
2020-01-16 10:19     ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 05/10] bpf: lsm: BTF API for LSM hooks KP Singh
2020-01-17  0:28   ` Andrii Nakryiko
2020-01-20 11:10     ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 06/10] bpf: lsm: Implement attach, detach and execution KP Singh
2020-01-15 17:24   ` Greg Kroah-Hartman
2020-01-16  9:45     ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 07/10] bpf: lsm: Make the allocated callback RO+X KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 08/10] tools/libbpf: Add support for BPF_PROG_TYPE_LSM KP Singh
2020-01-15 21:19   ` Andrii Nakryiko
2020-01-15 21:37     ` Andrii Nakryiko
2020-01-16 12:49     ` KP Singh
2020-01-16 17:26       ` KP Singh
2020-01-16 19:10       ` Andrii Nakryiko [this message]
2020-01-17 22:16         ` KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 09/10] bpf: lsm: Add selftests " KP Singh
2020-01-15 17:13 ` [PATCH bpf-next v2 10/10] bpf: lsm: Add Documentation KP Singh
2020-01-15 22:12 ` [PATCH bpf-next v2 00/10] MAC and Audit policy using eBPF (KRSI) Andrii Nakryiko
2020-01-20 11:12   ` KP Singh
2020-01-16 10:03 ` Brendan Jackman

Reply instructions:

You may reply publically 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=CAEf4BzYa+m0OCZXqnNsab+q3+HLboL45eRdBrGdZD+6Z4CRSiQ@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=christian@brauner.io \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jackmanb@chromium.org \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=joe@wand.net.nz \
    --cc=kafai@fb.com \
    --cc=keescook@chromium.org \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mchehab+samsung@kernel.org \
    --cc=mhalcrow@google.com \
    --cc=mic@digikod.net \
    --cc=mjg59@google.com \
    --cc=nicolas.ferre@microchip.com \
    --cc=pjt@google.com \
    --cc=quentin.monnet@netronome.com \
    --cc=rdna@fb.com \
    --cc=revest@chromium.org \
    --cc=sdf@google.com \
    --cc=serge@hallyn.com \
    --cc=songliubraving@fb.com \
    --cc=thgarnie@chromium.org \
    --cc=yhs@fb.com \
    /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

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git