Hello, David and I have been working on ~51 and this patch reflects changes that we've both committed. * Compaction of struct audit_watch and struct audit_wentry to struct audit_watch (David) * Compaction of struct audit_create_wentry and audit_create_watch (David / Tim) * The removal of the "drain watchlist on rename()" * Corrected argument net/socket.c:audit_socketcall() to prevent Oopsing * Added proper error handling in fs/namei.c (I think ;-)) * Fixed up spaces * Removed audit_drain_watch Two things left that I can think of: * Getting rid of blanket allocations of audit_inode_data * Allowing a _list_ of watches on a watched inode -> If the same inode is watched from multiple locations (or name spaces) * Removal of local reader-writer locks on audit_inode_data? -> I'm not sure that this lock is needed any longer now that there's only two ways we can remove from a watchlist and neither will contend with the other. (1) explicit administrative action, no other administrative action can occur at the same time (2) audit_inode_data is being freed, inode is no longer reachable for administrative action What do you all think? -tim diff -Nurp linux-2.6.9/fs/namei.c linux-2.6.9~working/fs/namei.c --- linux-2.6.9/fs/namei.c 2005-06-01 14:39:56.702915384 -0500 +++ linux-2.6.9~working/fs/namei.c 2005-06-01 13:16:11.000000000 -0500 @@ -213,7 +213,8 @@ int permission(struct inode * inode,int int retval; int submask; - audit_notify_watch(inode, mask); + if (audit_notify_watch(inode, mask)) + return -ENOMEM; /* Ordinary permission routines do not understand MAY_APPEND. */ submask = mask & ~MAY_APPEND; @@ -331,7 +332,8 @@ static inline int exec_permission_lite(s if (inode->i_op && inode->i_op->permission) return -EAGAIN; - audit_notify_watch(inode, MAY_EXEC); + if (audit_notify_watch(inode, MAY_EXEC)) + return -ENOMEM; if (current->fsuid == inode->i_uid) mode >>= 6; @@ -1113,7 +1115,9 @@ static inline int may_delete(struct inod BUG_ON(victim->d_parent->d_inode != dir); - audit_notify_watch(victim->d_inode, MAY_WRITE); + error = audit_notify_watch(victim->d_inode, MAY_WRITE); + if (error) + return error; error = permission(dir,MAY_WRITE | MAY_EXEC, NULL); if (error) @@ -1240,8 +1244,9 @@ int vfs_create(struct inode *dir, struct return error; DQUOT_INIT(dir); error = dir->i_op->create(dir, dentry, mode, nd); + if (!error) + error = audit_notify_watch(dentry->d_inode, MAY_WRITE); if (!error) { - audit_notify_watch(dentry->d_inode, MAY_WRITE); inode_dir_notify(dir, DN_CREATE); security_inode_post_create(dir, dentry, mode); } @@ -1555,8 +1560,9 @@ int vfs_mknod(struct inode *dir, struct DQUOT_INIT(dir); error = dir->i_op->mknod(dir, dentry, mode, dev); + if (!error) + error = audit_notify_watch(dentry->d_inode, MAY_WRITE); if (!error) { - audit_notify_watch(dentry->d_inode, MAY_WRITE); inode_dir_notify(dir, DN_CREATE); security_inode_post_mknod(dir, dentry, mode, dev); } @@ -1629,8 +1635,9 @@ int vfs_mkdir(struct inode *dir, struct DQUOT_INIT(dir); error = dir->i_op->mkdir(dir, dentry, mode); + if (!error) + error = audit_notify_watch(dentry->d_inode, MAY_WRITE); if (!error) { - audit_notify_watch(dentry->d_inode, MAY_WRITE); inode_dir_notify(dir, DN_CREATE); security_inode_post_mkdir(dir,dentry, mode); } @@ -1874,8 +1881,9 @@ int vfs_symlink(struct inode *dir, struc DQUOT_INIT(dir); error = dir->i_op->symlink(dir, dentry, oldname); + if (!error) + error = audit_notify_watch(dentry->d_inode, MAY_WRITE); if (!error) { - audit_notify_watch(dentry->d_inode, MAY_WRITE); inode_dir_notify(dir, DN_CREATE); security_inode_post_symlink(dir, dentry, oldname); } @@ -1948,8 +1956,9 @@ int vfs_link(struct dentry *old_dentry, DQUOT_INIT(dir); error = dir->i_op->link(old_dentry, dir, new_dentry); up(&old_dentry->d_inode->i_sem); + if (!error) + error = audit_notify_watch(new_dentry->d_inode, MAY_WRITE); if (!error) { - audit_notify_watch(new_dentry->d_inode, MAY_WRITE); inode_dir_notify(dir, DN_CREATE); security_inode_post_link(old_dentry, dir, new_dentry); } @@ -2071,12 +2080,13 @@ int vfs_rename_dir(struct inode *old_dir d_rehash(new_dentry); dput(new_dentry); } - if (!error) { + if (!error) d_move(old_dentry,new_dentry); - audit_notify_watch(old_dentry->d_inode, MAY_WRITE); + + error = audit_notify_watch(old_dentry->d_inode, MAY_WRITE); + if (!error) security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry); - } return error; } @@ -2102,9 +2112,12 @@ int vfs_rename_other(struct inode *old_d /* The following d_move() should become unconditional */ if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME)) d_move(old_dentry, new_dentry); - audit_notify_watch(old_dentry->d_inode, MAY_WRITE); - security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry); } + + error = audit_notify_watch(old_dentry->d_inode, MAY_WRITE); + if (!error) + security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry); + if (target) up(&target->i_sem); dput(new_dentry); diff -Nurp linux-2.6.9/include/linux/audit.h linux-2.6.9~working/include/linux/audit.h --- linux-2.6.9/include/linux/audit.h 2005-06-01 14:39:56.727911584 -0500 +++ linux-2.6.9~working/include/linux/audit.h 2005-06-01 14:18:08.000000000 -0500 @@ -215,32 +215,28 @@ struct watch_transport { #ifdef __KERNEL__ /* Structure associated with inode->i_audit */ -struct audit_inode_data { - struct audit_inode_data *next_hash; - struct inode *inode; - struct audit_wentry *wentry; - struct hlist_head watchlist; - rwlock_t lock; -}; - struct audit_watch { - dev_t dev; /* Superblock device */ - __u32 perms; /* Permissions filtering */ - char *name; /* Watch point beneath parent */ - char *path; /* Insertion path */ - char *filterkey; /* An arbitrary filtering key */ -}; - -struct audit_wentry { - struct rcu_head w_rcu; - struct hlist_node w_node; - struct hlist_node w_master; - struct audit_watch *w_watch; - struct dentry *w_dentry; atomic_t w_count; + struct hlist_node w_node; /* per-directory list */ + struct hlist_node w_master; /* Master watch list */ + struct dentry *w_dentry; /* Watched inode */ + dev_t w_dev; /* Superblock device */ + __u32 w_perms; /* Permissions filtering */ + 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 { + 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 */ + struct hlist_head watchlist; /* Watches for children */ + rwlock_t lock; }; + struct audit_sig_info { uid_t uid; pid_t pid; @@ -306,11 +302,11 @@ extern int audit_receive_watch(int type, 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_wentry_put(struct audit_wentry *wentry); +extern void audit_watch_put(struct audit_watch *watch); extern void audit_dentry_unpin(struct dentry *dentry); -extern struct audit_wentry *audit_wentry_get(struct audit_wentry *wentry); +extern struct audit_watch *audit_watch_get(struct audit_watch *watch); extern int audit_notify_watch(struct inode *inode, int mask); -extern int auditfs_attach_wdata(struct inode *inode, struct audit_wentry *wentry, +extern int auditfs_attach_wdata(struct inode *inode, struct audit_watch *watch, int mask); #else #define audit_filesystem_init() ({ 0; }) @@ -319,8 +315,8 @@ extern int auditfs_attach_wdata(struct i #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_wentry_put(w) do { ; } while(0) -#define audit_wentry_get(w) ({ 0; }) +#define audit_watch_put(w) do { ; } while(0) +#define audit_watch_get(w) ({ 0; }) #define audit_notify_watch(i,m) ({ 0; }) #endif diff -Nurp linux-2.6.9/init/Kconfig linux-2.6.9~working/init/Kconfig --- linux-2.6.9/init/Kconfig 2005-06-01 14:39:56.710914168 -0500 +++ linux-2.6.9~working/init/Kconfig 2005-06-01 15:02:04.703028680 -0500 @@ -169,6 +169,16 @@ config AUDITSYSCALL can be used independently or with another kernel subsystem, such as SELinux. +config AUDITFILESYSTEM + bool "Enable file system auditing support" + depends on AUDITSYSCALL + default n + help + Enable file system auditing for regular files and directories. + When a targeted file or directory is accessed, an audit record + is generated describing the inode accessed, how it was accessed, + and by whom (ie: pid and system call). + config LOG_BUF_SHIFT int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" if DEBUG_KERNEL range 12 20 @@ -186,16 +196,6 @@ config LOG_BUF_SHIFT 13 => 8 KB 12 => 4 KB -config AUDITFILESYSTEM - bool "Enable file system auditing support" - depends on AUDITSYSCALL - default n - help - Enable file system auditing for regular files and directories. - When a targeted file or directory is accessed, an audit record - is generated describing the inode accessed, how it was accessed, - and by whom (ie: pid and system call). - config HOTPLUG bool "Support for hot-pluggable devices" if !ARCH_S390 default ARCH_S390 diff -Nurp linux-2.6.9/kernel/auditfs.c linux-2.6.9~working/kernel/auditfs.c --- linux-2.6.9/kernel/auditfs.c 2005-06-01 14:39:56.755907328 -0500 +++ linux-2.6.9~working/kernel/auditfs.c 2005-06-01 14:22:46.000000000 -0500 @@ -40,16 +40,15 @@ extern int audit_enabled; static kmem_cache_t *audit_watch_cache; -static kmem_cache_t *audit_wentry_cache; /* Read-heavy list */ -HLIST_HEAD(master_watchlist); +static HLIST_HEAD(master_watchlist); static spinlock_t master_watchlist_lock = SPIN_LOCK_UNLOCKED; struct audit_skb_list { - struct hlist_node list; - void *memblk; - size_t size; + struct hlist_node list; + void *memblk; + size_t size; }; @@ -91,46 +90,46 @@ struct audit_inode_data *inode_audit_dat /* Private Interface */ -/* Unpin the dentry stored at wentry->w_dentry. */ -static inline void audit_unpin(struct audit_wentry *wentry) +/* Unpin the dentry stored at watch->w_dentry. */ +static inline void audit_unpin(struct audit_watch *watch) { - if (wentry && wentry->w_dentry) { - dput(wentry->w_dentry); - wentry->w_dentry = NULL; + if (watch && watch->w_dentry) { + dput(watch->w_dentry); + watch->w_dentry = NULL; } } -/* Pin the dentry and store it at wentry->w_dentry. */ -static inline void audit_pin(struct audit_wentry *wentry, +/* Pin the dentry and store it at watch->w_dentry. */ +static inline void audit_pin(struct audit_watch *watch, struct dentry *dentry) { - if (wentry && !wentry->w_dentry) - wentry->w_dentry = dget(dentry); + if (watch && !watch->w_dentry) + watch->w_dentry = dget(dentry); } -static inline struct audit_wentry *audit_wentry_fetch(const char *name, - struct audit_inode_data *data) +static inline struct audit_watch *audit_watch_fetch(const char *name, + struct audit_inode_data *data) { - struct audit_wentry *wentry, *ret = NULL; + struct audit_watch *watch, *ret = NULL; struct hlist_node *pos; - hlist_for_each_entry(wentry, pos, &data->watchlist, w_node) - if(!strcmp(wentry->w_watch->name, name)) { - ret = audit_wentry_get(wentry); + hlist_for_each_entry(watch, pos, &data->watchlist, w_node) + if(!strcmp(watch->w_name, name)) { + ret = audit_watch_get(watch); break; } return ret; } -static inline struct audit_wentry *audit_wentry_fetch_lock(const char *name, +static inline struct audit_watch *audit_watch_fetch_lock(const char *name, struct audit_inode_data *data) { - struct audit_wentry *ret = NULL; + struct audit_watch *ret = NULL; if (name && data) { read_lock(&data->lock); - ret = audit_wentry_fetch(name, data); + ret = audit_watch_fetch(name, data); read_unlock(&data->lock); } @@ -143,25 +142,37 @@ static inline struct audit_watch *audit_ watch = kmem_cache_alloc(audit_watch_cache, GFP_KERNEL); if (watch) { - watch->name = NULL; - watch->path = NULL; - watch->filterkey = NULL; - watch->perms = 0; + memset(watch, 0, sizeof(*watch)); + atomic_set(&watch->w_count, 1); } return watch; } +/* + * If a watch has been removed from a watchlist, we promptly unpin the + * dentry that was being watched, as it is no longer important to us. + */ static inline void audit_watch_free(struct audit_watch *watch) { if (watch) { - kfree(watch->name); - kfree(watch->path); - kfree(watch->filterkey); + audit_unpin(watch); + kfree(watch->w_name); + kfree(watch->w_path); + kfree(watch->w_filterkey); + 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); } } + /* Convert a watch_transport structure into a kernel audit_watch structure. */ static inline struct audit_watch *audit_to_watch(void *memblk) { @@ -173,23 +184,23 @@ static inline struct audit_watch *audit_ if (!watch) goto audit_to_watch_exit; - t = (struct watch_transport *)memblk; + t = memblk; - watch->perms = t->perms; + watch->w_perms = t->perms; offset = sizeof(struct watch_transport); - watch->filterkey = kmalloc(t->fklen+1, GFP_KERNEL); - if (!watch->filterkey) + watch->w_filterkey = kmalloc(t->fklen+1, GFP_KERNEL); + if (!watch->w_filterkey) goto audit_to_watch_fail; - watch->filterkey[t->fklen] = 0; - memcpy(watch->filterkey, memblk + offset, t->fklen); + watch->w_filterkey[t->fklen] = 0; + memcpy(watch->w_filterkey, memblk + offset, t->fklen); offset += t->fklen; - watch->path = kmalloc(t->pathlen+1, GFP_KERNEL); - if (!watch->path) + watch->w_path = kmalloc(t->pathlen+1, GFP_KERNEL); + if (!watch->w_path) goto audit_to_watch_fail; - watch->path[t->pathlen] = 0; - memcpy(watch->path, memblk + offset, t->pathlen); + watch->w_path[t->pathlen] = 0; + memcpy(watch->w_path, memblk + offset, t->pathlen); goto audit_to_watch_exit; @@ -206,99 +217,31 @@ audit_to_watch_exit: */ static inline void *audit_to_transport(struct audit_watch *watch, size_t size) { - unsigned int offset; - struct watch_transport t; - void *memblk; + struct watch_transport *t; + char *p; - memblk = kmalloc(size, GFP_ATOMIC); - if (!memblk) + t = kmalloc(size, GFP_KERNEL); + if (!t) goto audit_to_transport_exit; - memset(&t, 0, sizeof(t)); - - t.dev_major = MAJOR(watch->dev); - t.dev_minor = MINOR(watch->dev); - t.perms = watch->perms; - t.pathlen = strlen(watch->path) + 1; - if (watch->filterkey) - t.fklen = strlen(watch->filterkey) + 1; - - memcpy(memblk, &t, sizeof(t)); - offset = sizeof(t); - memcpy(memblk + offset, watch->filterkey, t.fklen); - offset += t.fklen; - memcpy(memblk + offset, watch->path, t.pathlen); - -audit_to_transport_exit: - return memblk; -} - -static inline struct audit_watch *audit_create_watch(const char *path, - const char *name, - const char *filterkey, - __u32 perms, dev_t dev) -{ - struct audit_watch *err = NULL; - struct audit_watch *watch = NULL; - - err = ERR_PTR(-ENOMEM); - watch = audit_watch_alloc(); - if (watch) { - watch->path = kmalloc(strlen(path)+1, GFP_KERNEL); - if (!watch->path) - goto audit_create_watch_fail; - strcpy(watch->path, path); - - watch->name = kmalloc(strlen(name)+1, GFP_KERNEL); - if (!watch->name) - goto audit_create_watch_fail; - strcpy(watch->name, name); - - if (filterkey) { - watch->filterkey = kmalloc(strlen(filterkey)+1, - GFP_KERNEL); - if (!watch->filterkey) - goto audit_create_watch_fail; - strcpy(watch->filterkey, filterkey); - } - - watch->dev = dev; - watch->perms = perms; - - goto audit_create_watch_exit; - } + memset(t, 0, sizeof(*t)); -audit_create_watch_fail: - audit_watch_free(watch); - watch = err; -audit_create_watch_exit: - return watch; -} + t->dev_major = MAJOR(watch->w_dev); + t->dev_minor = MINOR(watch->w_dev); + t->perms = watch->w_perms; + t->pathlen = strlen(watch->w_path) + 1; -static inline struct audit_wentry *audit_wentry_alloc(void) -{ - struct audit_wentry *wentry; + p = (char *)&t[1]; - wentry = kmem_cache_alloc(audit_wentry_cache, GFP_KERNEL); - if (wentry) { - atomic_set(&wentry->w_count, 1); - wentry->w_watch = NULL; - wentry->w_dentry = NULL; + if (watch->w_filterkey) { + t->fklen = strlen(watch->w_filterkey) + 1; + memcpy(p, watch->w_filterkey, t->fklen); + p += t->fklen; } + memcpy(p, watch->w_path, t->pathlen); - return wentry; -} -/* - * If a watch has been removed from a watchlist, we promptly unpin the - * dentry that was being watched, as it is no longer important to us. - */ -static inline void audit_wentry_free(struct audit_wentry *wentry) -{ - if (wentry) { - audit_unpin(wentry); - audit_watch_free(wentry->w_watch); - kmem_cache_free(audit_wentry_cache, wentry); - } +audit_to_transport_exit: + return t; } /* @@ -307,114 +250,92 @@ static inline void audit_wentry_free(str * watchlist and the local watchlist to ensure that insertions and removals of * watches are seralized. */ -static inline int audit_create_wentry(const char *path, - const char *name, - const char *filterkey, - __u32 perms, dev_t dev, - struct audit_inode_data *data) +static inline int audit_create_watch(const struct audit_watch *tmp, + struct audit_inode_data *data) { int ret; - struct audit_wentry *wentry = NULL; - struct audit_wentry *new = NULL; struct audit_watch *watch; ret = -EEXIST; - wentry = audit_wentry_fetch_lock(name, data); - if (wentry) { - audit_wentry_put(wentry); - goto audit_create_wentry_exit; + watch = audit_watch_fetch_lock(tmp->w_name, data); + if (watch) { + audit_watch_put(watch); + goto audit_create_watch_exit; } ret = -ENOMEM; - new = audit_wentry_alloc(); - if (!new) - goto audit_create_wentry_exit; - - watch = audit_create_watch(path, name, filterkey, perms, dev); - if (IS_ERR(watch)) { - ret = PTR_ERR(watch); - audit_wentry_put(new); - goto audit_create_wentry_exit; + watch = audit_watch_alloc(); + if (!watch) + goto audit_create_watch_exit; + + watch->w_path = kmalloc(strlen(tmp->w_path)+1, GFP_KERNEL); + if (!watch->w_path) + goto audit_create_watch_fail; + strcpy(watch->w_path, tmp->w_path); + + watch->w_name = kmalloc(strlen(tmp->w_name)+1, GFP_KERNEL); + if (!watch->w_name) + goto audit_create_watch_fail; + strcpy(watch->w_name, tmp->w_name); + + if (tmp->w_filterkey) { + watch->w_filterkey = kmalloc(strlen(tmp->w_filterkey)+1, GFP_KERNEL); + if (!watch->w_filterkey) + goto audit_create_watch_fail; + strcpy(watch->w_filterkey, tmp->w_filterkey); } - ret = 0; + watch->w_dev = tmp->w_dev; + watch->w_perms = tmp->w_perms; - new->w_watch = watch; - audit_wentry_get(new); + audit_watch_get(watch); write_lock(&data->lock); - hlist_add_head(&new->w_node, &data->watchlist); - spin_lock(&master_watchlist_lock); - hlist_add_head_rcu(&new->w_master, &master_watchlist); - spin_unlock(&master_watchlist_lock); + hlist_add_head(&watch->w_node, &data->watchlist); + spin_lock(&master_watchlist_lock); + hlist_add_head(&watch->w_master, &master_watchlist); + spin_unlock(&master_watchlist_lock); write_unlock(&data->lock); + return 0; -audit_create_wentry_exit: - return ret; -} - -static inline void audit_wentry_rcu_put(struct rcu_head *head) -{ - struct audit_wentry *wentry; + audit_create_watch_fail: + audit_watch_put(watch); - wentry = container_of(head, struct audit_wentry, w_rcu); - audit_wentry_put(wentry); + audit_create_watch_exit: + return ret; } -/* - * There's three ways we can arrive at audit_destroy_wentry. Two of the ways - * may cause contention with one another, while the third may not. They are: - * - * 1) An administrator has explicitly requested a watch removal - * 2) A directory with an "active" watchlist is rename()'ed - * 3) The inode is in the process of being destroyed - * - * In ways 1) and 2) the data->lock must be held to safely enter. We use - * the audit_master_watchlist spinlock to protect against removal of two - * consecutive entries concurrently in the master watchlist. - * - * If way 3) occurs there will no longer be any reachable references to the - * inode, thus there is only one way to remove this wentry, and no need to - * hold the data->lock. - */ -static inline void audit_destroy_wentry(struct audit_wentry *wentry) +static inline void audit_destroy_watch(struct audit_watch *watch) { - if (wentry) { - hlist_del_init(&wentry->w_node); - audit_wentry_put(wentry); - spin_lock(&master_watchlist_lock); - hlist_del_rcu(&wentry->w_master); - call_rcu(&wentry->w_rcu, audit_wentry_rcu_put); - spin_unlock(&master_watchlist_lock); + if (watch) { + hlist_del_init(&watch->w_node); + spin_lock(&master_watchlist_lock); + hlist_del_init(&watch->w_master); + spin_unlock(&master_watchlist_lock); + audit_watch_put(watch); + audit_watch_put(watch); } } - -/* - * 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 inline void audit_data_free(struct audit_inode_data *data) { - struct audit_wentry *wentry; + struct audit_watch *watch; + struct audit_inode_data *wdata = NULL; struct hlist_node *pos, *tmp; - hlist_for_each_entry_safe(wentry, pos, tmp, &data->watchlist, w_node) - audit_destroy_wentry(wentry); -} - + hlist_for_each_entry_safe(watch, pos, tmp, &data->watchlist, w_node) + audit_destroy_watch(watch); -static inline void audit_data_free(struct audit_inode_data *data) -{ - if (data) { - audit_drain_watchlist(data); - audit_wentry_put(data->wentry); - kfree(data); + /* 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); } + if (data != wdata) + kfree(data); + else + memset(data, 0, sizeof(*data)); } static inline int audit_insert_watch(struct audit_watch *watch, uid_t loginuid) @@ -422,20 +343,25 @@ static inline int audit_insert_watch(str int ret; struct nameidata nd; - ret = path_lookup(watch->path, LOOKUP_PARENT, &nd); + ret = path_lookup(watch->w_path, LOOKUP_PARENT, &nd); if (ret < 0) goto audit_insert_watch_exit; + /* FIXME: Allocate w_iaudit */ + ret = -EPERM; if (nd.last_type != LAST_NORM || !nd.last.name) goto audit_insert_watch_release; - ret = audit_create_wentry(watch->path, - nd.last.name, - watch->filterkey, - watch->perms, - nd.dentry->d_inode->i_sb->s_dev, - inode_audit_data(nd.dentry->d_inode)); + ret = -ENOMEM; + watch->w_name = kmalloc(strlen(nd.last.name)+1, GFP_KERNEL); + if (!watch->w_name) + goto audit_insert_watch_release; + strcpy(watch->w_name, nd.last.name); + + watch->w_dev = nd.dentry->d_inode->i_sb->s_dev; + + ret = audit_create_watch(watch, inode_audit_data(nd.dentry->d_inode)); if (ret < 0) goto audit_insert_watch_release; @@ -449,20 +375,14 @@ audit_insert_watch_exit: 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; struct nameidata nd; struct audit_inode_data *data; - struct audit_wentry *wentry; + struct audit_watch *real; - ret = path_lookup(watch->path, LOOKUP_PARENT, &nd); + ret = path_lookup(watch->w_path, LOOKUP_PARENT, &nd); if (ret < 0) goto audit_remove_watch_exit; @@ -473,13 +393,13 @@ static inline int audit_remove_watch(str data = inode_audit_data(nd.dentry->d_inode); write_lock(&data->lock); - wentry = audit_wentry_fetch(nd.last.name, data); - if (!wentry) { + real = audit_watch_fetch(nd.last.name, data); + if (!real) { write_unlock(&data->lock); goto audit_remove_watch_release; } - audit_destroy_wentry(wentry); - audit_wentry_put(wentry); + audit_destroy_watch(real); + audit_watch_put(real); audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u removed watch\n", loginuid); write_unlock(&data->lock); @@ -491,20 +411,20 @@ audit_remove_watch_exit: return ret; } -struct audit_wentry *audit_wentry_get(struct audit_wentry *wentry) +struct audit_watch *audit_watch_get(struct audit_watch *watch) { - if (wentry) { - BUG_ON(!atomic_read(&wentry->w_count)); - atomic_inc(&wentry->w_count); + if (watch) { + BUG_ON(!atomic_read(&watch->w_count)); + atomic_inc(&watch->w_count); } - return wentry; + return watch; } -void audit_wentry_put(struct audit_wentry *wentry) +void audit_watch_put(struct audit_watch *watch) { - if (wentry && atomic_dec_and_test(&wentry->w_count)) - audit_wentry_free(wentry); + if (watch && atomic_dec_and_test(&watch->w_count)) + audit_watch_free(watch); } /* @@ -522,7 +442,7 @@ void audit_wentry_put(struct audit_wentr */ void audit_update_watch(struct dentry *dentry, int remove) { - struct audit_wentry *wentry; + struct audit_watch *watch; struct audit_inode_data *data, *parent; if (likely(!audit_enabled)) @@ -542,29 +462,32 @@ void audit_update_watch(struct dentry *d } parent = inode_audit_data(dentry->d_parent->d_inode); - wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent); + watch = audit_watch_fetch_lock(dentry->d_name.name, parent); write_lock(&data->lock); /* FIXME: long watchlist == too much spinning? */ if (remove) { - audit_drain_watchlist(data); - if ((wentry && data->wentry)) - if (!strcmp(wentry->w_watch->name, - data->wentry->w_watch->name)) { - audit_wentry_put(data->wentry); - data->wentry = NULL; + /* If the inode was being watched for _this_ pathname, clear the watch */ + /* FIXME: If there was _another_ path to the same inode which was + supposed to be watched, we ought to continue watching. */ + if (watch && data->watch && + !strcmp(watch->w_name, data->watch->w_name)) { + audit_watch_put(data->watch); + data->watch = NULL; } - } else if (!data->wentry) { - data->wentry = audit_wentry_get(wentry); - audit_pin(data->wentry, dentry); - } else if (hlist_unhashed(&data->wentry->w_node)) { - audit_wentry_put(data->wentry); - data->wentry = audit_wentry_get(wentry); - audit_pin(data->wentry, dentry); + } else if (!data->watch) { + /* Inode wasn't watched before. Maybe it is now */ + data->watch = audit_watch_get(watch); + audit_pin(data->watch, dentry); + } else if (hlist_unhashed(&data->watch->w_node)) { + /* Old watch is dead now. Drop it and add new one .*/ + audit_watch_put(data->watch); + data->watch = audit_watch_get(watch); + audit_pin(data->watch, dentry); } write_unlock(&data->lock); - audit_wentry_put(wentry); + audit_watch_put(watch); } /* Convert a watch to a audit_skb_list */ @@ -575,15 +498,15 @@ struct audit_skb_list *audit_to_skb(stru struct audit_skb_list *entry; /* We must include space for both "\0" */ - size = sizeof(struct watch_transport) + strlen(watch->path) + - strlen(watch->filterkey) + 2; + size = sizeof(struct watch_transport) + strlen(watch->w_path) + + strlen(watch->w_filterkey) + 2; entry = ERR_PTR(-ENOMEM); memblk = audit_to_transport(watch, size); if (!memblk) goto audit_queue_watch_exit; - entry = kmalloc(sizeof(*entry), GFP_ATOMIC); + entry = kmalloc(sizeof(*entry), GFP_KERNEL); if (!entry) { entry = ERR_PTR(-ENOMEM); goto audit_queue_watch_exit; @@ -601,7 +524,7 @@ audit_queue_watch_exit: * file system and send it to user space. There will never be concurrent * readers of this list. * - * The reference to wentry will not be put back during a read upon a + * The reference to watch will not be put back during a read upon a * watch removal, until after we're done reading. So, the potential * for the rug being pulled out from under us is NIL. * @@ -613,21 +536,42 @@ int audit_list_watches(int pid, int seq) struct hlist_head skb_list; struct hlist_node *tmp, *pos; struct audit_skb_list *entry; - struct audit_wentry *wentry; + struct audit_watch *watch; + restart: INIT_HLIST_HEAD(&skb_list); + spin_lock(&master_watchlist_lock); - rcu_read_lock(); - hlist_for_each_entry_rcu(wentry, pos, &master_watchlist, w_master) { - entry = audit_to_skb(wentry->w_watch); + hlist_for_each_entry(watch, pos, &master_watchlist, w_master) { + audit_watch_get(watch); + spin_unlock(&master_watchlist_lock); + entry = audit_to_skb(watch); if (IS_ERR(entry)) { ret = PTR_ERR(entry); - rcu_read_unlock(); + spin_unlock(&master_watchlist_lock); goto audit_list_watches_fail; } hlist_add_head(&entry->list, &skb_list); + spin_lock(&master_watchlist_lock); + if (hlist_unhashed(&watch->w_master)) { + /* This watch was removed from the list while we + pondered it. We could play tricks to find how far + we'd got, but we might as well just start again + from scratch. There's no real chance of livelock, + as the number of watches in the system has + decreased, and the netlink sem prevents new watches + from being added while we're looping */ + audit_watch_put(watch); + hlist_for_each_entry_safe(entry, pos, tmp, &skb_list, list) { + hlist_del(&entry->list); + kfree(entry->memblk); + kfree(entry); + } + goto restart; + } + audit_watch_put(watch); } - rcu_read_unlock(); + spin_unlock(&master_watchlist_lock); hlist_for_each_entry_safe(entry, pos, tmp, &skb_list, list) { audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 0, 1, @@ -655,6 +599,7 @@ int audit_receive_watch(int type, int pi { int ret = 0; struct audit_watch *watch = NULL; + char *payload = (char *)&req[1]; ret = -EINVAL; if (req->pathlen > PATH_MAX || req->pathlen == 0) @@ -663,6 +608,9 @@ int audit_receive_watch(int type, int pi if (req->fklen > AUDIT_FILTERKEY_MAX) goto audit_receive_watch_exit; + if ( payload[req->fklen] != '/') + goto audit_receive_watch_exit; + if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND)) goto audit_receive_watch_exit; @@ -698,7 +646,7 @@ int audit_inode_alloc(struct inode *inod return -ENOMEM; INIT_HLIST_HEAD(&data->watchlist); - data->wentry = NULL; + data->watch = NULL; data->lock = RW_LOCK_UNLOCKED; data->inode = inode; data->next_hash = NULL; @@ -751,15 +699,15 @@ void audit_inode_free(struct inode *inod void audit_dentry_unpin(struct dentry *dentry) { struct audit_inode_data *parent; - struct audit_wentry *wentry; + struct audit_watch *watch; parent = inode_audit_data(dentry->d_parent->d_inode); - wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent); - if (wentry) { + watch = audit_watch_fetch_lock(dentry->d_name.name, parent); + if (watch) { struct audit_inode_data *data = inode_audit_data(dentry->d_inode); - audit_unpin(data->wentry); - audit_wentry_put(wentry); + audit_unpin(data->watch); + audit_watch_put(watch); } } @@ -774,12 +722,6 @@ int audit_filesystem_init(void) if (!audit_watch_cache) goto audit_filesystem_init_fail; - audit_wentry_cache = - kmem_cache_create("audit_wentry_cache", - sizeof(struct audit_wentry), 0, 0, NULL, NULL); - if (!audit_wentry_cache) - goto audit_filesystem_init_fail; - /* Set up hash table for inode objects */ auditfs_hash_bits = long_log2(auditfs_cache_buckets); if (auditfs_cache_buckets != (1 << auditfs_hash_bits)) { @@ -804,7 +746,6 @@ int audit_filesystem_init(void) audit_filesystem_init_fail: kmem_cache_destroy(audit_watch_cache); - kmem_cache_destroy(audit_wentry_cache); audit_filesystem_init_exit: return ret; } @@ -814,7 +755,7 @@ int audit_notify_watch(struct inode *ino { int ret = 0; struct audit_inode_data *data; - struct audit_wentry *wentry = NULL; + struct audit_watch *watch = NULL; if (likely(!audit_enabled)) return 0; @@ -827,24 +768,24 @@ int audit_notify_watch(struct inode *ino return 0; /* - * Won't be able to drop i_audit->wentry reference + * Won't be able to drop i_audit->watch reference * before we obtain our own reference */ read_lock(&data->lock); - wentry = audit_wentry_get(data->wentry); + watch = audit_watch_get(data->watch); read_unlock(&data->lock); - if (!wentry) + if (!watch) return 0; - if (mask && (wentry->w_watch->perms && !(wentry->w_watch->perms&mask))) + if (mask && (watch->w_perms && !(watch->w_perms & mask))) goto audit_notify_watch_fail; - ret = auditfs_attach_wdata(inode, wentry, mask); + ret = auditfs_attach_wdata(inode, watch, mask); if (!ret) return 0; audit_notify_watch_fail: - audit_wentry_put(wentry); + audit_watch_put(watch); return ret; } diff -Nurp linux-2.6.9/kernel/auditsc.c linux-2.6.9~working/kernel/auditsc.c --- linux-2.6.9/kernel/auditsc.c 2005-06-01 14:39:56.721912496 -0500 +++ linux-2.6.9~working/kernel/auditsc.c 2005-06-01 09:33:06.000000000 -0500 @@ -131,7 +131,7 @@ struct audit_aux_data_path { struct audit_aux_data_watched { struct audit_aux_data link; - struct audit_wentry *wentry; + struct audit_watch *watch; unsigned long ino; int mask; uid_t uid; @@ -586,7 +586,7 @@ static inline void audit_free_aux(struct break; } case AUDIT_FS_WATCH: { struct audit_aux_data_watched *axi = (void *)aux; - audit_wentry_put(axi->wentry); + audit_watch_put(axi->watch); break; } } @@ -769,13 +769,13 @@ static void audit_log_exit(struct audit_ case AUDIT_FS_WATCH: { struct audit_aux_data_watched *axi = (void *)aux; audit_log_format(ab, " watch="); - audit_log_untrustedstring(ab, axi->wentry->w_watch->name); + audit_log_untrustedstring(ab, axi->watch->w_name); audit_log_format(ab, " filterkey=%s perm=%u perm_mask=%d" " inode=%lu inode_uid=%u inode_gid=%u" " inode_dev=%02x:%02x inode_rdev=%02x:%02x", - axi->wentry->w_watch->filterkey, - axi->wentry->w_watch->perms, + axi->watch->w_filterkey, + axi->watch->w_perms, axi->mask, axi->ino, axi->uid, axi->gid, MAJOR(axi->dev), MINOR(axi->dev), MAJOR(axi->rdev), MINOR(axi->rdev)); @@ -1198,7 +1198,7 @@ void audit_signal_info(int sig, struct t #ifdef CONFIG_AUDITFILESYSTEM /* This has to be here instead of in auditfs.c, because it needs to see the audit context */ -int auditfs_attach_wdata(struct inode *inode, struct audit_wentry *wentry, +int auditfs_attach_wdata(struct inode *inode, struct audit_watch *watch, int mask) { struct audit_context *context = current->audit_context; @@ -1211,7 +1211,7 @@ int auditfs_attach_wdata(struct inode *i if (context->in_syscall && !context->auditable) context->auditable = 1; - ax->wentry = wentry; + ax->watch = watch; ax->mask = mask; ax->ino = inode->i_ino; ax->uid = inode->i_uid; diff -Nurp linux-2.6.9/net/socket.c linux-2.6.9~working/net/socket.c --- linux-2.6.9/net/socket.c 2005-06-01 14:39:56.606929976 -0500 +++ linux-2.6.9~working/net/socket.c 2005-06-01 09:33:22.000000000 -0500 @@ -1929,7 +1929,7 @@ asmlinkage long sys_socketcall(int call, if (copy_from_user(a, args, nargs[call])) return -EFAULT; - err = audit_socketcall(nargs[call]/sizeof(unsigned long), args); + err = audit_socketcall(nargs[call]/sizeof(unsigned long), a); if (err) return err;