[RFC][PATCH] auditfs userspace

[Steve Grubb]

On Tuesday 08 March 2005 10:14, Timothy R. Chavez wrote:
> These values come from include/linux/fs.h and I thought it'd be easiest 
> just to mirror them in userspace by copying them over.

I don't think its a good idea to mirror them. We'll move them to the right 
place and fix the includes.

But there's still a question from the last note...do we need a fs_enable? We 
don't have a rule enable. Its either got rules and produces output or it 
doesn't. (Ignore the fact that SE Linux avc messages get sent even with no 
rules.)

In auditsc.c is this chunk of code:

+       if (!auditfs_enabled)
+               return;
+
+       list_for_each_entry(file, &context->wtrail, list) {
+               ab = audit_log_start(context);
+               if (!ab)
+                       continue;

list_for_each_entry should avoid the loop if there's no rules. Therefore its 
less efficient to add a check for auditfs_enabled.

+            audit_log_format(ab,
+                   "name=%s filter_key=%s perm_mask=%u perm=%d inode=%lu "
+                   "inode_mode=%d inode_uid=%d inode_gid=%d "
+                   "inode_dev=%02x:%02x inode_rdev=%02x:%02x",
+                   file->wentry->w_watch->name,
+                   file->wentry->w_watch->filterkey,
+                   file->wentry->w_watch->perms,
+                   file->mask, file->ino, file->mode, file->uid,
+                   file->gid, MAJOR(file->dev), MINOR(file->dev),
+                   MAJOR(file->rdev), MINOR(file->rdev));

Are perm_mask & perm variables in the wrong order?

I have questions about audit_wentry_free - shouldn't wentry be zero'ed when 
its free instead of pointing to freed memory? I see a lot of code that 
branches if wentry is non-zero. Is the way freeing works racy? Is the list 
locked (I think I see 2 different locks)? Can other threads try to reference 
a partially deleted audit_wentry?

What about the name audit_wentry_put? How about audit_wentry_decr_ref? put 
sounds like a name that is inserting things, not potentially freeing them. 
Since it sometimes frees things, doesn't this leak memory:

+                       audit_wentry_put(data->wentry);
+                       data->wentry = NULL;

The zeroing should be atomic and in sync with reference counts.

-Steve