LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Kees Cook <keescook@chromium.org>
Cc: Tyler Hicks <tyhicks@canonical.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Eric Paris <eparis@redhat.com>,
	Steve Grubb <sgrubb@redhat.com>, Jonathan Corbet <corbet@lwn.net>,
	Linux Audit <linux-audit@redhat.com>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v2 4/4] seccomp: Don't special case audited processes when logging
Date: Wed, 2 May 2018 22:32:55 -0400	[thread overview]
Message-ID: <CAHC9VhQNaYrZeA487DyhuKGc-2GcydFC2Fr7vUrpoDPhbi4v4A@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jLnD=g=LxR5KTrvcwr1qaYiYXbH-ersvhs5NPRR6JA91w@mail.gmail.com>

On Wed, May 2, 2018 at 12:57 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, May 2, 2018 at 8:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index da78835..9029d9d 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -584,18 +584,13 @@ static inline void seccomp_log(unsigned long syscall, long signr, u32 action,
>>         }
>>
>>         /*
>> -        * Force an audit message to be emitted when the action is RET_KILL_*,
>> -        * RET_LOG, or the FILTER_FLAG_LOG bit was set and the action is
>> -        * allowed to be logged by the admin.
>> +        * Emit an audit message when the action is RET_KILL_*, RET_LOG, or the
>> +        * FILTER_FLAG_LOG bit was set. The admin has the ability to silence
>> +        * any action from being logged by removing the action name from the
>> +        * seccomp_actions_logged sysctl.
>>          */
>>         if (log)
>> -               return __audit_seccomp(syscall, signr, action);
>> -
>> -       /*
>> -        * Let the audit subsystem decide if the action should be audited based
>> -        * on whether the current task itself is being audited.
>> -        */
>> -       return audit_seccomp(syscall, signr, action);
>> +               audit_seccomp(syscall, signr, action);
>>  }
>
> This whole series looks great to me. If I can get an Ack from Paul for
> the audit bits, I can take it via the seccomp tree.

I got stuck doing some day job things today and didn't get a chance to
look at v2 today, but I'll do that tomorrow.

I'm not sure if you've already got anything queued up in your seccomp
tree Kees, but if not I'm happy to take these patches via the audit
tree; that was my original plan, assuming I got a positive nod from
you.  Either way, as long as it hits Linus' tree eventually I'll be
happy (assuming everything looks good post-review of course).

> ... One minor nit on
> seccomp_log() above, I'd probably change this to show the "exception"
> case as "out of line" of normal code flow. i.e. instead of "if (log)
> audit_seccomp", invert it to return early:
>
> ...
>     if (!log)
>         return;
>
>     audit_seccomp(syscall, signr, action);
> }
>
> But if there isn't some other need for a v3, I can just make this
> change when I commit.
>
> Thanks for fixing this up!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security



-- 
paul moore
www.paul-moore.com

      reply	other threads:[~2018-05-03  2:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02 15:53 [PATCH v2 0/4] Better integrate seccomp logging and auditing Tyler Hicks
2018-05-02 15:53 ` [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl Tyler Hicks
2018-05-02 21:01   ` James Morris
2018-05-02 15:53 ` [PATCH v2 2/4] seccomp: Configurable separator for the actions_logged string Tyler Hicks
2018-05-02 21:11   ` James Morris
2018-05-02 15:53 ` [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl Tyler Hicks
2018-05-02 18:18   ` Steve Grubb
2018-05-03 20:18     ` Paul Moore
2018-05-03 20:42       ` Steve Grubb
2018-05-03 20:48         ` Paul Moore
2018-05-03 20:51           ` Tyler Hicks
2018-05-03 21:12             ` Steve Grubb
2018-05-03 22:36               ` Tyler Hicks
2018-05-03 23:18                 ` Steve Grubb
2018-05-02 21:17   ` James Morris
2018-05-02 15:53 ` [PATCH v2 4/4] seccomp: Don't special case audited processes when logging Tyler Hicks
2018-05-02 16:57   ` Kees Cook
2018-05-03  2:32     ` Paul Moore [this message]

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=CAHC9VhQNaYrZeA487DyhuKGc-2GcydFC2Fr7vUrpoDPhbi4v4A@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=corbet@lwn.net \
    --cc=eparis@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=sgrubb@redhat.com \
    --cc=tyhicks@canonical.com \
    --cc=wad@chromium.org \
    --subject='Re: [PATCH v2 4/4] seccomp: Don'\''t special case audited processes when logging' \
    /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

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).