Re: [PATCH 1/1] audit: Add new syscalls to the perm=w filter

[Steve Grubb <sgrubb@redhat.com>]

On Monday, October 16, 2017 3:10:59 PM EDT Paul Moore wrote:
> On Thu, Oct 12, 2017 at 11:24 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > The audit subsystem allows selecting audit events based on watches for
> > a particular behavior like writing to a file. A lot of syscalls have
> > been added without updating the list. This patch adds 2 syscalls to the
> > write filters: fallocate and renameat2.
> > 
> > Signed-off-by: sgrubb <sgrubb@redhat.com>
> > ---
> > 
> >  include/asm-generic/audit_dir_write.h | 4 ++++
> >  include/asm-generic/audit_write.h     | 3 +++
> >  2 files changed, 7 insertions(+)
> 
> FWIW, I expect that this syscall list is almost always going to be out
> of date; it's just the way this feature is designed.  That doesn't
> mean I'm not going to merge fixes, I just want to make sure
> expectations are set accordingly.

I understand...but we are years behind. I just wanted to close the gap on a 
couple obvious syscalls since everyone else is busy with more important bugs. 
I have not done an indepth review, so there may still be others. For example, 
do we consider time stamp of a file one of its attributes? If so, then 
utimensat may need to be brought in. I also have no idea what to make of 
name_to_handle_at and open_by_handle_at.


> Before I merge this Steve, can you explain why fallocate() should be
> on the write list? 

Its a whole lot like ftruncate.

$ touch gh
$ ls -l gh
-rw-rw-r--. 1 sgrubb sgrubb 0 Oct 16 16:21 gh

strace fallocate -o 0 -l 1024 -z gh
<snip>
open("gh", O_RDWR)                      = 3
fallocate(3, FALLOC_FL_ZERO_RANGE, 0, 1024) = 0
fsync(3)                                = 0
close(3)                                = 0
<snip>
$ ls -l gh
-rw-rw-r--. 1 sgrubb sgrubb 1024 Oct 16 16:21 gh

File went from 0 bytes to 1024 bytes.


> It doesn't actually write any user data to disk, it actually doesn't write
> anything, all it does is play with the amount of space allocated for the
> given fd on the storage device. 

It has a bunch of flags which can zeroize holes or extents.

> I don't really care either way, this just struck me as odd and I want to
> make sure you have a good reason (hint: add it to the patch
> description).

Understandable. But its close enough to ftruncate that I think it qualifies.

-Steve

> > diff --git a/include/asm-generic/audit_dir_write.h
> > b/include/asm-generic/audit_dir_write.h index 7b61db4fe72b..d9a53eaeea35
> > 100644
> > --- a/include/asm-generic/audit_dir_write.h
> > +++ b/include/asm-generic/audit_dir_write.h
> > @@ -30,3 +30,7 @@ __NR_renameat,
> > 
> >  __NR_linkat,
> >  __NR_symlinkat,
> >  #endif
> > 
> > +#ifdef __NR_renameat2
> > +__NR_renameat2,
> > +#endif
> > +
> > diff --git a/include/asm-generic/audit_write.h
> > b/include/asm-generic/audit_write.h index 274575d7129f..4fa65816acd3
> > 100644
> > --- a/include/asm-generic/audit_write.h
> > +++ b/include/asm-generic/audit_write.h
> > @@ -19,3 +19,6 @@ __NR_ftruncate64,
> > 
> >  #ifdef __NR_bind
> >  __NR_bind,             /* bind can affect fs object only in one way... */
> >  #endif
> > 
> > +#ifdef __NR_fallocate
> > +__NR_fallocate,
> > +#endif