Re: patch update to ~51

On Thu, 2005-06-02 at 12:03 +0100, David Woodhouse wrote: --- linux-2.6.9/include/linux/audit.h.p20057 2005-06-02 18:02:20.000000000 +0100 +++ linux-2.6.9/include/linux/audit.h 2005-06-02 18:02:20.000000000 +0100 @@ -225,10 +225,10 @@ struct audit_watch { char *w_name; /* Watch point beneath parent */ char *w_path; /* Insertion path */ char *w_filterkey; /* An arbitrary filtering key */ - struct audit_inode_data *w_iaudit; /* Preallocated inode data */ }; struct audit_inode_data { + int count; struct audit_inode_data *next_hash; /* Watch data hash table */ struct inode *inode; /* Inode to which it belongs */ struct audit_watch *watch; /* Watch for this inode */ @@ -299,7 +299,6 @@ extern int audit_filesystem_init(void); extern int audit_list_watches(int pid, int seq); extern int audit_receive_watch(int type, int pid, int uid, int seq, struct watch_transport *req, uid_t loginuid); -extern int audit_inode_alloc(struct inode *inode); extern void audit_inode_free(struct inode *inode); extern void audit_update_watch(struct dentry *dentry, int remove); extern void audit_watch_put(struct audit_watch *watch); @@ -312,7 +311,6 @@ extern int auditfs_attach_wdata(struct i #define audit_filesystem_init() ({ 0; }) #define audit_list_watches(p,s) ({ -EOPNOTSUPP; }) #define audit_receive_watch(t,p,u,s,r,l) ({ -EOPNOTSUPP; }) -#define audit_inode_alloc(i) ({ 0; }) #define audit_inode_free(i) do { ; } while(0) #define audit_update_watch(d,r) do { ; } while (0) #define audit_watch_put(w) do { ; } while(0) --- linux-2.6.9/fs/inode.c.p20057 2005-06-02 18:02:20.000000000 +0100 +++ linux-2.6.9/fs/inode.c 2005-06-02 18:02:20.000000000 +0100 @@ -137,8 +137,7 @@ static struct inode *alloc_inode(struct inode->i_rdev = 0; inode->i_security = NULL; inode->dirtied_when = 0; - if (audit_inode_alloc(inode) || security_inode_alloc(inode)) { - audit_inode_free(inode); + if (security_inode_alloc(inode)) { if (inode->i_sb->s_op->destroy_inode) inode->i_sb->s_op->destroy_inode(inode); else --- linux-2.6.9/kernel/auditfs.c.p20057 2005-06-02 18:02:20.000000000 +0100 +++ linux-2.6.9/kernel/auditfs.c 2005-06-02 18:02:20.000000000 +0100 @@ -36,6 +36,14 @@ #include <linux/module.h> #include <asm/uaccess.h> +#if 1 +#define dprintk(...) do { } while(0) +#define __print_symbol(x, y) do { } while(0) +#else +#define dprintk(...) printk(KERN_DEBUG __VA_ARGS__); +extern void __print_symbol(char *, void *); +#define inline +#endif extern int audit_enabled; @@ -52,6 +60,9 @@ struct audit_skb_list { }; +static int audit_nr_watches; +static int audit_pool_size; +static struct audit_inode_data *audit_data_pool; static struct audit_inode_data **auditfs_hash_table; static spinlock_t auditfs_hash_lock = SPIN_LOCK_UNLOCKED; static int auditfs_hash_bits; @@ -59,32 +70,73 @@ static int auditfs_cache_buckets = 16384 module_param(auditfs_cache_buckets, int, 0); MODULE_PARM_DESC(auditfs_cache_buckets, "Number of auditfs cache entries to allocate (default 16384)\n"); -struct audit_inode_data *inode_audit_data(struct inode *inode) +static int audit_data_pool_grow(void) +{ + struct audit_inode_data *new; + + new = kmalloc(sizeof(*new), GFP_KERNEL); + if (!new) + return -ENOMEM; + new->next_hash = kmalloc(sizeof(*new), GFP_KERNEL); + if (!new->next_hash) { + kfree(new); + return -ENOMEM; + } + + spin_lock(&auditfs_hash_lock); + new->next_hash->next_hash = audit_data_pool; + audit_data_pool = new; + audit_nr_watches++; + audit_pool_size += 2; + spin_unlock(&auditfs_hash_lock); + return 0; +} +static void audit_data_pool_shrink(void) +{ + spin_lock(&auditfs_hash_lock); + audit_nr_watches--; + spin_unlock(&auditfs_hash_lock); +} + +static struct audit_inode_data *audit_data_get(struct inode *inode, int allocate) { struct audit_inode_data **list; - int h = hash_ptr(inode, auditfs_hash_bits); struct audit_inode_data *ret = NULL; + int h = hash_ptr(inode, auditfs_hash_bits); list = &auditfs_hash_table[h]; spin_lock(&auditfs_hash_lock); - while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) + while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) { + dprintk("list %p -> %p\n", list, *list); list = &(*list)->next_hash; - + } if (*list && (*list)->inode == inode) ret = *list; - if (!ret) { - /* Hm. At the moment, we should never see an inode which doesn't have audit data */ - printk("Hm. No audit data for inode %p. Hash bucket for hash %d follows...\n", inode, h); - list = &auditfs_hash_table[h]; - while (*list) { - printk("ino=%p data=%p\n", (*list)->inode, *list); - list = &(*list)->next_hash; - } + if (ret) { + ret->count++; + } else if (allocate) { + ret = audit_data_pool; + audit_data_pool = ret->next_hash; + audit_pool_size--; + dprintk("allocate from pool. %d left\n", audit_pool_size); + + INIT_HLIST_HEAD(&ret->watchlist); + ret->watch = NULL; + ret->lock = RW_LOCK_UNLOCKED; + ret->inode = inode; + ret->next_hash = *list; + ret->count = 2; + *list = ret; + } + if (ret) { + dprintk("Got audit data %p for inode %p (%lu), count++ now %d. From %p: ", + ret, ret->inode, ret->inode->i_ino, ret->count, __builtin_return_address(0)); + __print_symbol("%s\n", __builtin_return_address(0)); } - spin_unlock(&auditfs_hash_lock); + return ret; } @@ -108,7 +160,7 @@ static inline void audit_pin(struct audi } static inline struct audit_watch *audit_watch_fetch(const char *name, - struct audit_inode_data *data) + struct audit_inode_data *data) { struct audit_watch *watch, *ret = NULL; struct hlist_node *pos; @@ -123,7 +175,7 @@ static inline struct audit_watch *audit_ } static inline struct audit_watch *audit_watch_fetch_lock(const char *name, - struct audit_inode_data *data) + struct audit_inode_data *data) { struct audit_watch *ret = NULL; @@ -163,11 +215,6 @@ static inline void audit_watch_free(stru BUG_ON(watch->w_dentry); BUG_ON(!hlist_unhashed(&watch->w_node)); BUG_ON(!hlist_unhashed(&watch->w_master)); - if (watch->w_iaudit) { - BUG_ON(watch->w_iaudit->inode); - BUG_ON(!hlist_empty(&watch->w_iaudit->watchlist)); - kfree(watch->w_iaudit); - } kmem_cache_free(audit_watch_cache, watch); } } @@ -248,7 +295,7 @@ audit_to_transport_exit: * Because we're protected by audit_netlink_sem, there will never be a race on * current insertions of the same watch. However, we do lock both the master * watchlist and the local watchlist to ensure that insertions and removals of - * watches are seralized. + * watches are serialized. */ static inline int audit_create_watch(const char *path, const char *name, @@ -336,83 +383,111 @@ static inline void audit_destroy_watch(s } } - -/* - * We only drain the watchlist in two ways. They are: - * - * 1) Upon deletion of the inode (no contention) - * 2) Upon rename()'ing a directory with an "active" watchlist (contention) - * - * Caller should hold data->lock where contention is possible. - */ -static inline void audit_drain_watchlist(struct audit_inode_data *data) +static void audit_data_put(struct audit_inode_data *data) { - struct audit_watch *watch; - struct hlist_node *pos, *tmp; + if (!data) + return; - hlist_for_each_entry_safe(watch, pos, tmp, &data->watchlist, w_node) - audit_destroy_watch(watch); -} + spin_lock(&auditfs_hash_lock); + data->count--; + dprintk("Put audit_data %p for inode %p (%lu), count-- now %d. From %p:", data, + data->inode, data->inode?data->inode->i_ino:0, data->count, __builtin_return_address(0)); + __print_symbol("%s\n", __builtin_return_address(0)); + + if (data->count == 1 && data->inode && + !data->watch && hlist_empty(&data->watchlist)) { + dprintk("Last put.\n"); + data->count--; + } + + if (!data->count) { + /* We are last user. Remove it from the hash table to + disassociate it from its inode */ + struct audit_watch *watch; + struct hlist_node *pos, *tmp; + int h = hash_ptr(data->inode, auditfs_hash_bits); + struct audit_inode_data **list = &auditfs_hash_table[h]; -static inline void audit_data_free(struct audit_inode_data *data) -{ - struct audit_inode_data *wdata = NULL; + while (*list && (unsigned long)((*list)->inode) < (unsigned long)data->inode) + list = &(*list)->next_hash; - audit_drain_watchlist(data); + BUG_ON(*list != data); + *list = data->next_hash; + data->inode = NULL; - /* If this inode was watched itself, we may have been using - the preallocated inode audit data. So don't free it */ - if (data->watch) { - wdata = data->watch->w_iaudit; - audit_watch_put(data->watch); + spin_unlock(&auditfs_hash_lock); + + /* Drain watchlist */ + hlist_for_each_entry_safe(watch, pos, tmp, &data->watchlist, w_node) { + audit_destroy_watch(watch); + audit_data_pool_shrink(); + } + spin_lock(&auditfs_hash_lock); + + /* Check whether to free it or return it to the pool */ + if (audit_nr_watches > audit_pool_size) { + dprintk("Back to pool. %d watches, %d in pool\n", audit_nr_watches, audit_pool_size); + data->next_hash = audit_data_pool; + audit_data_pool = data; + audit_pool_size++; + } else { + dprintk("Freed. %d watches, %d in pool\n", audit_nr_watches, audit_pool_size); + kfree(data); + } } - if (data != wdata) - kfree(data); - else - memset(data, 0, sizeof(*data)); + + spin_unlock(&auditfs_hash_lock); } static inline int audit_insert_watch(struct audit_watch *watch, uid_t loginuid) { int ret; struct nameidata nd; + struct audit_inode_data *pdata; + + /* Grow the pool by two -- one for the watch itself, and + one for the parent directory */ + if (audit_data_pool_grow()) + return -ENOMEM; ret = path_lookup(watch->w_path, LOOKUP_PARENT, &nd); if (ret < 0) - goto audit_insert_watch_exit; - - /* FIXME: Allocate w_iaudit */ + goto out; ret = -EPERM; if (nd.last_type != LAST_NORM || !nd.last.name) - goto audit_insert_watch_release; + goto release; + + ret = -ENOMEM; + pdata = audit_data_get(nd.dentry->d_inode, 1); + if (!pdata) + goto put_pdata; ret = audit_create_watch(watch->w_path, nd.last.name, watch->w_filterkey, watch->w_perms, nd.dentry->d_inode->i_sb->s_dev, - inode_audit_data(nd.dentry->d_inode)); + pdata); if (ret < 0) - goto audit_insert_watch_release; + goto put_pdata; audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u inserted watch\n", loginuid); /* __d_lookup will attach the audit data, if nd.last exists. */ dput(d_lookup(nd.dentry, &nd.last)); -audit_insert_watch_release: + put_pdata: + audit_data_put(pdata); + release: path_release(&nd); -audit_insert_watch_exit: + out: + if (ret) + audit_data_pool_shrink(); + return ret; } -/* - * We hold the data->lock here to eliminate contention between this user space - * request and an action taken by audit_update_watch() in which watch removal - * is also possible when a directory with an "active" watchlist is rename()'ed. - * - */ static inline int audit_remove_watch(struct audit_watch *watch, uid_t loginuid) { int ret; @@ -428,7 +503,9 @@ static inline int audit_remove_watch(str if (nd.last_type != LAST_NORM || !nd.last.name) goto audit_remove_watch_release; - data = inode_audit_data(nd.dentry->d_inode); + data = audit_data_get(nd.dentry->d_inode, 0); + if (!data) + goto audit_remove_watch_release; write_lock(&data->lock); real = audit_watch_fetch(nd.last.name, data); @@ -440,7 +517,8 @@ static inline int audit_remove_watch(str audit_watch_put(real); audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u removed watch\n", loginuid); write_unlock(&data->lock); - + audit_data_put(data); + audit_data_pool_shrink(); ret = 0; audit_remove_watch_release: @@ -480,7 +558,7 @@ void audit_watch_put(struct audit_watch */ void audit_update_watch(struct dentry *dentry, int remove) { - struct audit_watch *watch; + struct audit_watch *watch = NULL; struct audit_inode_data *data, *parent; if (likely(!audit_enabled)) @@ -492,23 +570,25 @@ void audit_update_watch(struct dentry *d if (!dentry->d_parent || !dentry->d_parent->d_inode) return; - data = inode_audit_data(dentry->d_inode); - if (!data) { - printk(KERN_WARNING "Hmmm. No audit data for inode #%lu at %p. name %s\n", - dentry->d_inode->i_ino, dentry->d_inode, dentry->d_name.name); + /* If there's no audit data on the parent inode, then there can + be no watches to add or remove */ + parent = audit_data_get(dentry->d_parent->d_inode, 0); + if (!parent) return; - } - parent = inode_audit_data(dentry->d_parent->d_inode); watch = audit_watch_fetch_lock(dentry->d_name.name, parent); + /* Fetch audit data, using the preallocated one from the watch if + there is actually a relevant watch and the inode didn't already + have any audit data */ + data = audit_data_get(dentry->d_inode, !!watch); + + /* If there's no data, then there wasn't a watch either. + Nothing to see here; move along */ + if (!data) + goto put_watch; + write_lock(&data->lock); - /* FIXME: long watchlist == too much spinning? */ - if (remove == 1) { - /* Inode actually being removed. If it was a directory, drain - its watchlist too. */ - audit_drain_watchlist(data); - } if (remove) { /* If the inode was being watched for _this_ pathname, clear the watch */ /* FIXME: If there was _another_ path to the same inode which was @@ -528,9 +608,12 @@ void audit_update_watch(struct dentry *d data->watch = audit_watch_get(watch); audit_pin(data->watch, dentry); } + /* FIXME: If inode's i_audit is no longer used, clear it. */ write_unlock(&data->lock); - + audit_data_put(data); + put_watch: audit_watch_put(watch); + audit_data_put(parent); } /* Convert a watch to a audit_skb_list */ @@ -678,58 +761,16 @@ audit_receive_watch_exit: return ret; } -int audit_inode_alloc(struct inode *inode) -{ - struct audit_inode_data *data; - struct audit_inode_data **list; - int h = hash_ptr(inode, auditfs_hash_bits); - - data = kmalloc(sizeof(struct audit_inode_data), GFP_KERNEL); - if (!data) - return -ENOMEM; - - INIT_HLIST_HEAD(&data->watchlist); - data->watch = NULL; - data->lock = RW_LOCK_UNLOCKED; - data->inode = inode; - data->next_hash = NULL; - - /* Add it to the hash table */ - list = &auditfs_hash_table[h]; - spin_lock(&auditfs_hash_lock); - - while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) - list = &(*list)->next_hash; - - data->next_hash = *list; - *list = data; - - spin_unlock(&auditfs_hash_lock); - return 0; -} - void audit_inode_free(struct inode *inode) { - struct audit_inode_data *data = NULL; - int h = hash_ptr(inode, auditfs_hash_bits); - struct audit_inode_data **list; + struct audit_inode_data *data = audit_data_get(inode, 0); - if (!inode) - return; - - spin_lock(&auditfs_hash_lock); - list = &auditfs_hash_table[h]; - while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) - list = &(*list)->next_hash; - - if (*list && (*list)->inode == inode) { - data = *list; - *list = data->next_hash; + if (data) { + audit_drain_watchlist(data); + audit_watch_put(data->watch); + data->watch = NULL; + audit_data_put(data); } - spin_unlock(&auditfs_hash_lock); - - if (data) - audit_data_free(data); } /* @@ -744,14 +785,21 @@ void audit_dentry_unpin(struct dentry *d struct audit_inode_data *parent; struct audit_watch *watch; - parent = inode_audit_data(dentry->d_parent->d_inode); + parent = audit_data_get(dentry->d_parent->d_inode, 0); + if (!parent) + return; + watch = audit_watch_fetch_lock(dentry->d_name.name, parent); if (watch) { - struct audit_inode_data *data = inode_audit_data(dentry->d_inode); + struct audit_inode_data *data = audit_data_get(dentry->d_inode, 0); - audit_unpin(data->watch); + if (data) { + audit_unpin(data->watch); + audit_data_put(data); + } audit_watch_put(watch); } + audit_data_put(parent); } int audit_filesystem_init(void) @@ -806,7 +854,7 @@ int audit_notify_watch(struct inode *ino if (!inode || !current->audit_context) return 0; - data = inode_audit_data(inode); + data = audit_data_get(inode, 0); if (!data) return 0; @@ -818,17 +866,18 @@ int audit_notify_watch(struct inode *ino watch = audit_watch_get(data->watch); read_unlock(&data->lock); if (!watch) - return 0; + goto out; if (mask && (watch->w_perms && !(watch->w_perms & mask))) - goto audit_notify_watch_fail; + goto fail; ret = auditfs_attach_wdata(inode, watch, mask); - if (!ret) - return 0; - - audit_notify_watch_fail: - audit_watch_put(watch); + if (ret) { + fail: + audit_watch_put(watch); + } + out: + audit_data_put(data); return ret; } -- dwmw2