[PATCH 2/2] filesystem auditing: augment audit_inode

[Amy Griffis]

On Thu, Oct 20, 2005 at 10:04:09AM -0500, Timothy R. Chavez wrote:
> > Not too many comments on my first cursory glance.

Thanks for taking a look.

> > diff --git a/fs/open.c b/fs/open.c
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/pagemap.h>
> >  #include <linux/syscalls.h>
> >  #include <linux/rcupdate.h>
> > +#include <linux/audit.h>
> >  
> >  #include <asm/unistd.h>
> >  
> > @@ -609,6 +610,8 @@ asmlinkage long sys_fchmod(unsigned int 
> >  	dentry = file->f_dentry;
> >  	inode = dentry->d_inode;
> >  
> > +	audit_inode(NULL, inode, 0);
> > +
> >  	err = -EROFS;
> >  	if (IS_RDONLY(inode))
> >  		goto out_putf;
> > @@ -732,7 +735,10 @@ asmlinkage long sys_fchown(unsigned int 
> >  
> >  	file = fget(fd);
> >  	if (file) {
> > -		error = chown_common(file->f_dentry, user, group);
> > +		struct dentry * dentry;
> 
> I guess for consistency's sake this should be declared at the top?
> 
> > +		dentry = file->f_dentry;
> > +		audit_inode(NULL, dentry->d_inode, 0);
> > +		error = chown_common(dentry, user, group);
> >  		fput(file);
> >  	}
> >  	return error;

Well, these two functions aren't really consistent between themselves
as is.  sys_fchmod checks for !file and jumps out early.  sys_fchown
encloses the code in a conditional block.  I'd prefer to keep the
declaration local to the block since it should give the compiler
better opportunities for optimization.  I don't think it's any less
readable either.

> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
<snip>
> > @@ -1202,13 +1217,93 @@ void audit_inode(const char *name, const
> >  		++context->ino_count;
> >  #endif
> >  	}
> > -	context->names[idx].flags = flags;
> > -	context->names[idx].ino   = inode->i_ino;
> >  	context->names[idx].dev	  = inode->i_sb->s_dev;
> >  	context->names[idx].mode  = inode->i_mode;
> >  	context->names[idx].uid   = inode->i_uid;
> >  	context->names[idx].gid   = inode->i_gid;
> >  	context->names[idx].rdev  = inode->i_rdev;
> > +	if ((flags & LOOKUP_PARENT) && (strcmp(name, "/") != 0) && 
> > +	    (strcmp(name, ".") != 0)) {
> > +		context->names[idx].ino   = (unsigned long)-1;
> > +		context->names[idx].pino  = inode->i_ino;
> > +	} else {
> > +		context->names[idx].ino   = inode->i_ino;
> > +		context->names[idx].pino  = (unsigned long)-1;
> > +	}
> > +}
> > +
> > +/**
> > + * audit_inode_child - collect inode info for created/removed objects
> > + * @dname: inode's dentry name
> > + * @inode: inode being audited
> > + * @pino: inode number of dentry parent
> > + *
> > + * For syscalls that create or remove filesystem objects, audit_inode
> > + * can only collect information for the filesystem object's parent.
> > + * This call updates the audit context with the child's information.
> > + * Syscalls that create a new filesystem object must be hooked after
> > + * the object is created.  Syscalls that remove a filesystem object
> > + * must be hooked prior, in order to capture the target inode during
> > + * unsuccessful attempts.
> > + */
> > +void __audit_inode_child(const char *dname, const struct inode *inode,
> > +			 unsigned long pino)
> > +{
> > +	int idx;
> > +	struct audit_context *context = current->audit_context;
> 
> Does this process even if !audit_enabled?

Nope.  The audit_inode_child wrapper in audit.h checks for
current->audit_context before calling __audit_inode_child.  You
won't have an audit_context when syscall auditing is disabled.

> 
> > +
> > +	if (!context->in_syscall)
> > +		return;
> > +