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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 5BCEFC432C0 for ; Wed, 4 Dec 2019 02:53:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 204092073B for ; Wed, 4 Dec 2019 02:53:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=paul-moore-com.20150623.gappssmtp.com header.i=@paul-moore-com.20150623.gappssmtp.com header.b="aI4SDOI6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726766AbfLDCxa (ORCPT ); Tue, 3 Dec 2019 21:53:30 -0500 Received: from mail-lf1-f54.google.com ([209.85.167.54]:43280 "EHLO mail-lf1-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726521AbfLDCxa (ORCPT ); Tue, 3 Dec 2019 21:53:30 -0500 Received: by mail-lf1-f54.google.com with SMTP id 9so3331446lfq.10 for ; Tue, 03 Dec 2019 18:53:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uR+pT65soPzjqJ67S48glzTfhGJEKMDDLWprJm5r4mY=; b=aI4SDOI62/pPrhGCaEj1/ngReuOGp6v0q67iB7ucEval3f3gGBEC8Dwizix4mEsV65 N+tG1o/mmvoQZf+pi2hrlpgY0lPat20YwANxIleHDULtynaUMm1rZjbVYDgr0dy14c7T 6AblvCluhJtUHCGaa/qoKfbUiNTmwF1dTPx/ovhs2d0knIpyEK7Rhwuuyqh4HKzno9e8 tLWrxYiPt4rX06K9qnvHHcXkf2qk4LpK2fIIEycQOrDRPsyLzKkSwdti5RkkJns/bNjn 5iBjr8wbkrUGIeaqsCg0Vr7D03tyB6rOOgjJEA2dH8vRR8pZrriFZRP2nA6gMe5XOP6j /nzQ== 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=uR+pT65soPzjqJ67S48glzTfhGJEKMDDLWprJm5r4mY=; b=WizJKA8i+82PrLa6XXBqYvhU+CVXXO/Tvt86Wn4vSP1OZjgCY6KsqFqK1SkC049R97 fjqVteh0L/Ugmu+twPqxfqddcQIEOUjZQbMIJTlFBgmdL8Q0NPHGHO+2lmIb2ABiqBUl dPGKEbWswUw4UHsYKamYmOErBqFgSTWT5Me+UB/kW+EPe4uAsTHJZESZ8MM5/K96Oz5j JY2LgPSktzPXs4gkePsWRunKnaSq+QX7gvC8HDe4ljpOjQ4dYArux+pN2gHe8N+5uuND 3H3VadnGFRbKwEshZlDZIChn+pDPTm4UcTYP4FHfccSDPGbFCMsjwd+n+rG2Mn7ePJKS OPgA== X-Gm-Message-State: APjAAAVdnTh1jEOfaMw30RsX4ddwIaRiodm8T4f07qEjtymCzBz8zj4W 2jSfvPOFPfHmxnY/Tqs+6b5uoUy3SBsNrz+PLlnY X-Google-Smtp-Source: APXvYqxC2jCV69k21E2WvCNty27MXD7mqixisctmZLTFJfdlz1LwQeP4SH3ptOANsNEsWPDaO4sPYI3Qqt1vdi3PFGA= X-Received: by 2002:ac2:424d:: with SMTP id m13mr619905lfl.13.1575428007737; Tue, 03 Dec 2019 18:53:27 -0800 (PST) MIME-Version: 1.0 References: <20191128091633.29275-1-jolsa@kernel.org> <20191203093837.GC17468@krava> In-Reply-To: <20191203093837.GC17468@krava> From: Paul Moore Date: Tue, 3 Dec 2019 21:53:16 -0500 Message-ID: Subject: Re: [RFC] bpf: Emit audit messages upon successful prog load and unload To: Jiri Olsa Cc: Jiri Olsa , Alexei Starovoitov , Daniel Borkmann , netdev@vger.kernel.org, bpf@vger.kernel.org, linux-audit@redhat.com, Andrii Nakryiko , Yonghong Song , Martin KaFai Lau , Jakub Kicinski , Steve Grubb , David Miller , Eric Paris , Jiri Benc Content-Type: text/plain; charset="UTF-8" Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Tue, Dec 3, 2019 at 4:38 AM Jiri Olsa wrote: > On Mon, Dec 02, 2019 at 06:00:14PM -0500, Paul Moore wrote: > > On Thu, Nov 28, 2019 at 4:16 AM Jiri Olsa wrote: ... > > > --- a/kernel/bpf/syscall.c > > > +++ b/kernel/bpf/syscall.c > > > @@ -23,6 +23,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > > > > #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \ > > > @@ -1306,6 +1307,30 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog) > > > return 0; > > > } > > > > > > +enum bpf_audit { > > > + BPF_AUDIT_LOAD, > > > + BPF_AUDIT_UNLOAD, > > > +}; > > > + > > > +static const char * const bpf_audit_str[] = { > > > + [BPF_AUDIT_LOAD] = "LOAD", > > > + [BPF_AUDIT_UNLOAD] = "UNLOAD", > > > +}; > > > + > > > +static void bpf_audit_prog(const struct bpf_prog *prog, enum bpf_audit op) > > > +{ > > > + struct audit_buffer *ab; > > > + > > > + if (audit_enabled == AUDIT_OFF) > > > + return; > > > > I think you would probably also want to check the results of > > audit_dummy_context() here as well, see all the various audit_XXX() > > functions in include/linux/audit.h as an example. You'll see a > > pattern similar to the following: > > > > static inline void audit_foo(...) > > { > > if (unlikely(!audit_dummy_context())) > > __audit_foo(...) > > } > > bpf_audit_prog might be called outside of syscall context for UNLOAD event, > so that would prevent it from being stored Okay, right. More on this below ... > I can see audit_log_start checks on value of audit_context() that we pass in, The check in audit_log_start() is for a different reason; it checks the passed context to see if it should associate the record with others in the same event, e.g. PATH records associated with the matching SYSCALL record. This way all the associated records show up as part of the same event (as defined by the audit timestamp). > should we check for audit_dummy_context just for load event? like: > > > static void bpf_audit_prog(const struct bpf_prog *prog, enum bpf_audit op) > { > struct audit_buffer *ab; > > if (audit_enabled == AUDIT_OFF) > return; > if (op == BPF_AUDIT_LOAD && audit_dummy_context()) > return; > ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_BPF); > if (unlikely(!ab)) > return; > ... > } Ignoring the dummy context for a minute, there is likely a larger issue here with using audit_context() when bpf_audit_prog() is called outside of a syscall, e.g. BPF_AUDIT_UNLOAD. In this case we likely shouldn't be taking the audit context from the current task, we shouldn't be taking it from anywhere, it should be NULL. As far as the dummy context is concerned, you might want to skip the dummy context check since you can only do that on the LOAD side, which means that depending on the system's configuration you could end up with a number of unbalanced LOAD/UNLOAD events. The downside is that you are always going to get the BPF audit records on systemd based systems, since they ignore the admin's audit configuration and always enable audit (yes, we've tried to get systemd to change, they don't seem to care). -- paul moore www.paul-moore.com