linux-kernel - Re: [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with anonymous parents

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhSb5Q8hSwvzvyY6rfZZDyC0NnOghokTf=FqFKXX6RUt9Q@mail.gmail.com>
Date:   Thu, 9 Nov 2017 10:18:10 -0500
From:   Paul Moore <paul@...l-moore.com>
To:     Steve Grubb <sgrubb@...hat.com>,
        Richard Guy Briggs <rgb@...hat.com>
Cc:     linux-audit@...hat.com, linux-kernel@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with
 anonymous parents

On Wed, Nov 8, 2017 at 6:29 PM, Steve Grubb <sgrubb@...hat.com> wrote:
> On Wednesday, September 20, 2017 12:52:32 PM EST Paul Moore wrote:
>> On Wed, Aug 23, 2017 at 7:03 AM, Richard Guy Briggs <rgb@...hat.com> wrote:
>> > Tracefs or debugfs were causing hundreds to thousands of null PATH
>> > records to be associated with the init_module and finit_module SYSCALL
>> > records on a few modules when the following rule was in place for
>> >
>> > startup:
>> >         -a always,exit -F arch=x86_64 -S init_module -F key=mod-load
>> >
>> > This happens because the parent inode is not found in the task's
>> > audit_names list and hence treats it as anonymous.  This gives us no
>> > information other than a numerical device number that may no longer be
>> > visible upon log inspeciton, and an inode number.
>> >
>> > Fill in the filesystem type, filesystem magic number and full pathname
>> > from the filesystem mount point on previously null PATH records from
>> > entries that have an anonymous parent from the child dentry using
>> > dentry_path_raw().
>
> Late reply...but I just noticed that this changes the format of the "name"
> field - which is undesirable. Please put the file system type in a field all
> by itself called "fstype". You can just leave it as the hex magic number
> prepended with 0x and user space can do the lookup from there,
>
> It might be simplest to just apply a corrective patch over top of this one so
> that you don't have to muck about with git branches and commit messages.

A quick note on the "corrective patch": given we are just days away
from the merge window opening, it is *way* to late for something like
that, at this point the only options are to leave it as-is or
yank/revert and make another pass during the next development phase.

As for the objection itself: ungh.  There is really no good reason why
you couldn't have seen this in the *several* *months* prior to this;
Richard wrote a nice patch description which *included* sample audit
events, and you were involved in discussions regarding this patchset.
To say I'm disappointed would be an understatement.

I need to look at the rest of audit/next to see what a mess things
would be if I yanked this patch.  I don't expect it to be bad, but
taking a look will also give Richard a chance to voice his thoughts;
it is his patch after all, it would be nice to see an "OK" from him.
Whatever we do, it needs to happen by the of the day today (Thursday,
November 9th) as we need time to build and test the revised patches.

-- 
paul moore
www.paul-moore.com