From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28994C33CB6 for ; Thu, 16 Jan 2020 19:11:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0182E206E6 for ; Thu, 16 Jan 2020 19:11:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="oAu9qaet" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387730AbgAPTLL (ORCPT ); Thu, 16 Jan 2020 14:11:11 -0500 Received: from mail-qv1-f68.google.com ([209.85.219.68]:40069 "EHLO mail-qv1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2437423AbgAPTLK (ORCPT ); Thu, 16 Jan 2020 14:11:10 -0500 Received: by mail-qv1-f68.google.com with SMTP id dp13so9590853qvb.7; Thu, 16 Jan 2020 11:11:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3OEZx2MjoszDi5bRMEvk6at9kqk898GlGBhsPS60YZE=; b=oAu9qaet49v37A1VgP0LaeMPw44TNjd+84DuWb1JRGRrE+ukvBC3pxHmRLEd9/cHlT DNiDww2RsL5wRM9Pah171TlZcCeGVVQh42LFK10TVFblyeHmSeu9P/z06XX2gy+UBiSb eFJF2CJfbOD2fWQK/Wg+I79B3/+W8MA2ruE2Bo5AfdR+le6U1yjWROe0+k5Kph9vB/yR 3OjJu+9TPR2me7OzueGwlEqPFXT+mgE3fGrk2kgeT6DiIwyFgZHgzn2If1rs/Fk/WT// q9DTjBW0P8MLUenVyMzJKX8iq/BMb9s9iIMhF+uoDtcfi9AlvQCWx5QSO/1OW4lhMcmf ji8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3OEZx2MjoszDi5bRMEvk6at9kqk898GlGBhsPS60YZE=; b=r8slrELBPZawiYrBSQRHUk2jaRRpAxbAoBRvlFOQtqDk0ILNHYzz1kzc+pXm17aabL uY4e0FWXKVfpNMYpPZVow7VBKMe53TmgZB8RvnExEp/oSWCSFUF201Pn5uA5SIcQ7Xym e9+SMpo9TX/ZPugUQblepRkL3+dS0aVoJtbm2+0Iz6C9EyajveOKayNj870XO3SxzTg1 MTqLlBPe8sgf4zKWkb3aOZp1YFu3XkUjfnSakapyvsv8/cEtCnmAJqaXn0yBUzbYcxdD E+kTRStXAHuLLJIxYWlrwWSIItGCrzTDAJRBdfFaODoTVeDcnWRtuuirL47AHD5hL+te T4GQ== X-Gm-Message-State: APjAAAUe6WwtDnutuLXDiuKD+UhXNpYv4j63pBzviJ3VJ3PSD3V4qqmA YC/V3vtkuClR/V+PoDOxOE3IzLATFrroelbErKA= X-Google-Smtp-Source: APXvYqw9FIgttSCmKhfwxg+gCy9NRY9si3en5ZFJS6VWOz3YlihoKUoN8mv6ZMAfLxb1JV1nrGcclF7WKhszpNDXPE8= X-Received: by 2002:ad4:514e:: with SMTP id g14mr4208129qvq.196.1579201868349; Thu, 16 Jan 2020 11:11:08 -0800 (PST) MIME-Version: 1.0 References: <20200115171333.28811-1-kpsingh@chromium.org> <20200115171333.28811-9-kpsingh@chromium.org> <20200116124955.GD240584@google.com> In-Reply-To: <20200116124955.GD240584@google.com> From: Andrii Nakryiko Date: Thu, 16 Jan 2020 11:10:57 -0800 Message-ID: Subject: Re: [PATCH bpf-next v2 08/10] tools/libbpf: Add support for BPF_PROG_TYPE_LSM To: KP Singh Cc: open list , bpf , linux-security-module@vger.kernel.org, Alexei Starovoitov , Daniel Borkmann , James Morris , Kees Cook , Thomas Garnier , Michael Halcrow , Paul Turner , Brendan Gregg , Jann Horn , Matthew Garrett , Christian Brauner , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= , Florent Revest , Brendan Jackman , Martin KaFai Lau , Song Liu , Yonghong Song , "Serge E. Hallyn" , Mauro Carvalho Chehab , "David S. Miller" , Greg Kroah-Hartman , Nicolas Ferre , Stanislav Fomichev , Quentin Monnet , Andrey Ignatov , Joe Stringer Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: On Thu, Jan 16, 2020 at 4:49 AM KP Singh 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 wrote: > > > > > > From: KP Singh > > > > > > * 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 > > > --- > > > 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. > [...]