[RFC][PATCH] (#7U4) file system auditing by location and name

[Timothy R. Chavez]

On Sun, 2005-05-08 at 09:51 -0400, Steve Grubb wrote:
>  static spinlock_t audit_master_watchlist_lock = SPIN_LOCK_UNLOCKED;
>  HLIST_HEAD(audit_master_watchlist);
> @@ -78,9 +78,19 @@
>  {
>         struct audit_watch *watch;
>  
> +       if (audit_watch_cache == NULL) {
> +               /* FIXME: not sure if this is racy */
> +               audit_watch_cache =
> kmem_cache_create("audit_watch_cache",
> +                               sizeof(struct audit_watch), 0, 0,
> NULL, NULL);
> +               if (audit_watch_cache == NULL) {
> +                       printk(KERN_ERR "Could not allocate
> audit_watch_cache");
> +                       return NULL;
> +               }
> +       }

I think we're cool here.  There are two ways we could arrive here:

audit_watch_alloc()

1) audit_receive_watch->audit_to_watch 
2) audit_insert_watch->audit_create_wentry->audit_create_watch

And we'll always hit #1 first (which is protected against contention
with audit_netlink_sem)

audit_wentry_alloc()

1) audit_insert_watch->audit_create_wentry

Because we're holding audit_netlink_sem we'll there will never be a
chance where two "inserts" race here either and because this data is not
accessible via the hooks yet, we won't have to worry about an internal
racing either.

Is that a fair analysis?

Admittedly, my approach was sloppier, and it appeared it wasn't working
for you (but I'm trying to figure out why still... in theory, the init
function should have always been hit prior to being able to add any kind
of watch right).  This approach is better, but I'm still not liking this
conditional (that's only ever going to execute its contents once).

Maybe we should be using unlikely() here?

-tim