From fa14a0b8d2bca1d2226afaa04bdf80200d8e9b03 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 22 Jan 2015 02:16:49 -0500 Subject: [PATCH 1/7] cut down the number of do_path_lookup() callers ... and don't bother with new struct filename when we already have one Signed-off-by: Al Viro --- fs/namei.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index bc35b02883bb..73fcf4280d6e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2046,7 +2046,8 @@ struct dentry *kern_path_locked(const char *name, struct path *path) { struct nameidata nd; struct dentry *d; - int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd); + struct filename filename = {.name = name}; + int err = filename_lookup(AT_FDCWD, &filename, LOOKUP_PARENT, &nd); if (err) return ERR_PTR(err); if (nd.last_type != LAST_NORM) { @@ -3290,7 +3291,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt, return file; } -struct dentry *kern_path_create(int dfd, const char *pathname, +static struct dentry *filename_create(int dfd, struct filename *name, struct path *path, unsigned int lookup_flags) { struct dentry *dentry = ERR_PTR(-EEXIST); @@ -3305,7 +3306,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname, */ lookup_flags &= LOOKUP_REVAL; - error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd); + error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd); if (error) return ERR_PTR(error); @@ -3359,6 +3360,13 @@ out: path_put(&nd.path); return dentry; } + +struct dentry *kern_path_create(int dfd, const char *pathname, + struct path *path, unsigned int lookup_flags) +{ + struct filename filename = {.name = pathname}; + return filename_create(dfd, &filename, path, lookup_flags); +} EXPORT_SYMBOL(kern_path_create); void done_path_create(struct path *path, struct dentry *dentry) @@ -3377,7 +3385,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname, struct dentry *res; if (IS_ERR(tmp)) return ERR_CAST(tmp); - res = kern_path_create(dfd, tmp->name, path, lookup_flags); + res = filename_create(dfd, tmp, path, lookup_flags); putname(tmp); return res; } From 08518549722f0c992a9e4be71a0777f37147e9d2 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 21 Jan 2015 23:59:56 -0500 Subject: [PATCH 2/7] fs: rework getname_kernel to handle up to PATH_MAX sized filenames In preparation for expanded use in the kernel, make getname_kernel() more useful by allowing it to handle any legal filename length. Thanks to Guenter Roeck for his suggestion to substitute memcpy() for strlcpy(). CC: linux@roeck-us.net CC: viro@zeniv.linux.org.uk CC: linux-fsdevel@vger.kernel.org Signed-off-by: Paul Moore Signed-off-by: Al Viro --- fs/namei.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 73fcf4280d6e..71dc1cc23b1a 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -212,32 +212,38 @@ getname(const char __user * filename) return getname_flags(filename, 0, NULL); } -/* - * The "getname_kernel()" interface doesn't do pathnames longer - * than EMBEDDED_NAME_MAX. Deal with it - you're a kernel user. - */ struct filename * getname_kernel(const char * filename) { struct filename *result; - char *kname; - int len; - - len = strlen(filename); - if (len >= EMBEDDED_NAME_MAX) - return ERR_PTR(-ENAMETOOLONG); + int len = strlen(filename) + 1; result = __getname(); if (unlikely(!result)) return ERR_PTR(-ENOMEM); - kname = (char *)result + sizeof(*result); - result->name = kname; + if (len <= EMBEDDED_NAME_MAX) { + result->name = (char *)(result) + sizeof(*result); + result->separate = false; + } else if (len <= PATH_MAX) { + struct filename *tmp; + + tmp = kmalloc(sizeof(*tmp), GFP_KERNEL); + if (unlikely(!tmp)) { + __putname(result); + return ERR_PTR(-ENOMEM); + } + tmp->name = (char *)result; + tmp->separate = true; + result = tmp; + } else { + __putname(result); + return ERR_PTR(-ENAMETOOLONG); + } + memcpy((char *)result->name, filename, len); result->uptr = NULL; result->aname = NULL; - result->separate = false; - strlcpy(kname, filename, EMBEDDED_NAME_MAX); return result; } From 5168910413830435fa3f0a593933a83721ec8bad Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 22 Jan 2015 00:00:03 -0500 Subject: [PATCH 3/7] fs: create proper filename objects using getname_kernel() There are several areas in the kernel that create temporary filename objects using the following pattern: int func(const char *name) { struct filename *file = { .name = name }; ... return 0; } ... which for the most part works okay, but it causes havoc within the audit subsystem as the filename object does not persist beyond the lifetime of the function. This patch converts all of these temporary filename objects into proper filename objects using getname_kernel() and putname() which ensure that the filename object persists until the audit subsystem is finished with it. Also, a special thanks to Al Viro, Guenter Roeck, and Sabrina Dubroca for helping resolve a difficult kernel panic on boot related to a use-after-free problem in kern_path_create(); the thread can be seen at the link below: * https://lkml.org/lkml/2015/1/20/710 This patch includes code that was either based on, or directly written by Al in the above thread. CC: viro@zeniv.linux.org.uk CC: linux@roeck-us.net CC: sd@queasysnail.net CC: linux-fsdevel@vger.kernel.org Signed-off-by: Paul Moore Signed-off-by: Al Viro --- fs/exec.c | 10 +++++++-- fs/namei.c | 64 ++++++++++++++++++++++++++++++++++++++++-------------- fs/open.c | 10 +++++++-- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index ad8798e26be9..c7f9b733406d 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -794,8 +794,14 @@ exit: struct file *open_exec(const char *name) { - struct filename tmp = { .name = name }; - return do_open_execat(AT_FDCWD, &tmp, 0); + struct filename *filename = getname_kernel(name); + struct file *f = ERR_CAST(filename); + + if (!IS_ERR(filename)) { + f = do_open_execat(AT_FDCWD, filename, 0); + putname(filename); + } + return f; } EXPORT_SYMBOL(open_exec); diff --git a/fs/namei.c b/fs/namei.c index 71dc1cc23b1a..0bc7742a591d 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2042,32 +2042,47 @@ static int filename_lookup(int dfd, struct filename *name, static int do_path_lookup(int dfd, const char *name, unsigned int flags, struct nameidata *nd) { - struct filename filename = { .name = name }; + struct filename *filename = getname_kernel(name); + int retval = PTR_ERR(filename); - return filename_lookup(dfd, &filename, flags, nd); + if (!IS_ERR(filename)) { + retval = filename_lookup(dfd, filename, flags, nd); + putname(filename); + } + return retval; } /* does lookup, returns the object with parent locked */ struct dentry *kern_path_locked(const char *name, struct path *path) { + struct filename *filename = getname_kernel(name); struct nameidata nd; struct dentry *d; - struct filename filename = {.name = name}; - int err = filename_lookup(AT_FDCWD, &filename, LOOKUP_PARENT, &nd); - if (err) - return ERR_PTR(err); + int err; + + if (IS_ERR(filename)) + return ERR_CAST(filename); + + err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd); + if (err) { + d = ERR_PTR(err); + goto out; + } if (nd.last_type != LAST_NORM) { path_put(&nd.path); - return ERR_PTR(-EINVAL); + d = ERR_PTR(-EINVAL); + goto out; } mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT); d = __lookup_hash(&nd.last, nd.path.dentry, 0); if (IS_ERR(d)) { mutex_unlock(&nd.path.dentry->d_inode->i_mutex); path_put(&nd.path); - return d; + goto out; } *path = nd.path; +out: + putname(filename); return d; } @@ -2399,8 +2414,14 @@ int kern_path_mountpoint(int dfd, const char *name, struct path *path, unsigned int flags) { - struct filename s = {.name = name}; - return filename_mountpoint(dfd, &s, path, flags); + struct filename *s = getname_kernel(name); + int retval = PTR_ERR(s); + + if (!IS_ERR(s)) { + retval = filename_mountpoint(dfd, s, path, flags); + putname(s); + } + return retval; } EXPORT_SYMBOL(kern_path_mountpoint); @@ -3280,7 +3301,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt, { struct nameidata nd; struct file *file; - struct filename filename = { .name = name }; + struct filename *filename; int flags = op->lookup_flags | LOOKUP_ROOT; nd.root.mnt = mnt; @@ -3289,11 +3310,16 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt, if (d_is_symlink(dentry) && op->intent & LOOKUP_OPEN) return ERR_PTR(-ELOOP); - file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_RCU); + filename = getname_kernel(name); + if (unlikely(IS_ERR(filename))) + return ERR_CAST(filename); + + file = path_openat(-1, filename, &nd, op, flags | LOOKUP_RCU); if (unlikely(file == ERR_PTR(-ECHILD))) - file = path_openat(-1, &filename, &nd, op, flags); + file = path_openat(-1, filename, &nd, op, flags); if (unlikely(file == ERR_PTR(-ESTALE))) - file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_REVAL); + file = path_openat(-1, filename, &nd, op, flags | LOOKUP_REVAL); + putname(filename); return file; } @@ -3370,8 +3396,14 @@ out: struct dentry *kern_path_create(int dfd, const char *pathname, struct path *path, unsigned int lookup_flags) { - struct filename filename = {.name = pathname}; - return filename_create(dfd, &filename, path, lookup_flags); + struct filename *filename = getname_kernel(pathname); + struct dentry *res; + + if (IS_ERR(filename)) + return ERR_CAST(filename); + res = filename_create(dfd, filename, path, lookup_flags); + putname(filename); + return res; } EXPORT_SYMBOL(kern_path_create); diff --git a/fs/open.c b/fs/open.c index 813be037b412..d36c42ff019d 100644 --- a/fs/open.c +++ b/fs/open.c @@ -971,8 +971,14 @@ struct file *file_open_name(struct filename *name, int flags, umode_t mode) */ struct file *filp_open(const char *filename, int flags, umode_t mode) { - struct filename name = {.name = filename}; - return file_open_name(&name, flags, mode); + struct filename *name = getname_kernel(filename); + struct file *file = ERR_CAST(name); + + if (!IS_ERR(name)) { + file = file_open_name(name, flags, mode); + putname(name); + } + return file; } EXPORT_SYMBOL(filp_open); From cbaab2db9103cc6727c7166d2fda9f64038c828c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 22 Jan 2015 02:49:00 -0500 Subject: [PATCH 4/7] simpler calling conventions for filename_mountpoint() a) make it accept ERR_PTR() as filename (and return its PTR_ERR() in that case) b) make it putname() the sucker in the end otherwise simplifies life for callers... Signed-off-by: Al Viro --- fs/namei.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 0bc7742a591d..5ec3515162e6 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2373,13 +2373,17 @@ static int filename_mountpoint(int dfd, struct filename *s, struct path *path, unsigned int flags) { - int error = path_mountpoint(dfd, s->name, path, flags | LOOKUP_RCU); + int error; + if (IS_ERR(s)) + return PTR_ERR(s); + error = path_mountpoint(dfd, s->name, path, flags | LOOKUP_RCU); if (unlikely(error == -ECHILD)) error = path_mountpoint(dfd, s->name, path, flags); if (unlikely(error == -ESTALE)) error = path_mountpoint(dfd, s->name, path, flags | LOOKUP_REVAL); if (likely(!error)) audit_inode(s, path->dentry, 0); + putname(s); return error; } @@ -2401,27 +2405,14 @@ int user_path_mountpoint_at(int dfd, const char __user *name, unsigned int flags, struct path *path) { - struct filename *s = getname(name); - int error; - if (IS_ERR(s)) - return PTR_ERR(s); - error = filename_mountpoint(dfd, s, path, flags); - putname(s); - return error; + return filename_mountpoint(dfd, getname(name), path, flags); } int kern_path_mountpoint(int dfd, const char *name, struct path *path, unsigned int flags) { - struct filename *s = getname_kernel(name); - int retval = PTR_ERR(s); - - if (!IS_ERR(s)) { - retval = filename_mountpoint(dfd, s, path, flags); - putname(s); - } - return retval; + return filename_mountpoint(dfd, getname_kernel(name), path, flags); } EXPORT_SYMBOL(kern_path_mountpoint); From fd3522fdc84023b050bb40318d9fc71a9adc22bc Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 22 Jan 2015 00:00:10 -0500 Subject: [PATCH 5/7] audit: enable filename recording via getname_kernel() Enable recording of filenames in getname_kernel() and remove the kludgy workaround in __audit_inode() now that we have proper filename logging for kernel users. CC: viro@zeniv.linux.org.uk CC: linux-fsdevel@vger.kernel.org Signed-off-by: Paul Moore Reviewed-by: Richard Guy Briggs Signed-off-by: Al Viro --- fs/namei.c | 1 + kernel/auditsc.c | 40 +++------------------------------------- 2 files changed, 4 insertions(+), 37 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 5ec3515162e6..a3fde77d4abf 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -243,6 +243,7 @@ getname_kernel(const char * filename) memcpy((char *)result->name, filename, len); result->uptr = NULL; result->aname = NULL; + audit_getname(result); return result; } diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 072566dd0caf..132dbcdef6ec 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1882,44 +1882,10 @@ out_alloc: n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN); if (!n) return; - /* unfortunately, while we may have a path name to record with the - * inode, we can't always rely on the string lasting until the end of - * the syscall so we need to create our own copy, it may fail due to - * memory allocation issues, but we do our best */ - if (name) { - /* we can't use getname_kernel() due to size limits */ - size_t len = strlen(name->name) + 1; - struct filename *new = __getname(); + if (name) + /* no need to set ->name_put as the original will cleanup */ + n->name = name; - if (unlikely(!new)) - goto out; - - if (len <= (PATH_MAX - sizeof(*new))) { - new->name = (char *)(new) + sizeof(*new); - new->separate = false; - } else if (len <= PATH_MAX) { - /* this looks odd, but is due to final_putname() */ - struct filename *new2; - - new2 = kmalloc(sizeof(*new2), GFP_KERNEL); - if (unlikely(!new2)) { - __putname(new); - goto out; - } - new2->name = (char *)new; - new2->separate = true; - new = new2; - } else { - /* we should never get here, but let's be safe */ - __putname(new); - goto out; - } - strlcpy((char *)new->name, name->name, len); - new->uptr = NULL; - new->aname = n; - n->name = new; - n->name_put = true; - } out: if (parent) { n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL; From 57c59f5837bdfd0b4fee3b02a44857e263a09bfa Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 22 Jan 2015 00:00:16 -0500 Subject: [PATCH 6/7] audit: fix filename matching in __audit_inode() and __audit_inode_child() In all likelihood there were some subtle, and perhaps not so subtle, bugs with filename matching in audit_inode() and audit_inode_child() for some time, however, recent changes to the audit filename code have definitely broken the filename matching code. The breakage could result in duplicate filenames in the audit log and other odd audit record entries. This patch fixes the filename matching code and restores some sanity to the filename audit records. CC: viro@zeniv.linux.org.uk CC: linux-fsdevel@vger.kernel.org Signed-off-by: Paul Moore Signed-off-by: Al Viro --- kernel/auditsc.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 132dbcdef6ec..4f521964ccaa 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1846,6 +1846,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry, /* The struct filename _must_ have a populated ->name */ BUG_ON(!name->name); #endif + /* * If we have a pointer to an audit_names entry already, then we can * just use it directly if the type is correct. @@ -1863,7 +1864,17 @@ void __audit_inode(struct filename *name, const struct dentry *dentry, } list_for_each_entry_reverse(n, &context->names_list, list) { - if (!n->name || strcmp(n->name->name, name->name)) + if (n->ino) { + /* valid inode number, use that for the comparison */ + if (n->ino != inode->i_ino || + n->dev != inode->i_sb->s_dev) + continue; + } else if (n->name) { + /* inode number has not been set, check the name */ + if (strcmp(n->name->name, name->name)) + continue; + } else + /* no inode and no name (?!) ... this is odd ... */ continue; /* match the correct record type */ @@ -1936,11 +1947,16 @@ void __audit_inode_child(const struct inode *parent, /* look for a parent entry first */ list_for_each_entry(n, &context->names_list, list) { - if (!n->name || n->type != AUDIT_TYPE_PARENT) + if (!n->name || + (n->type != AUDIT_TYPE_PARENT && + n->type != AUDIT_TYPE_UNKNOWN)) continue; - if (n->ino == parent->i_ino && - !audit_compare_dname_path(dname, n->name->name, n->name_len)) { + if (n->ino == parent->i_ino && n->dev == parent->i_sb->s_dev && + !audit_compare_dname_path(dname, + n->name->name, n->name_len)) { + if (n->type == AUDIT_TYPE_UNKNOWN) + n->type = AUDIT_TYPE_PARENT; found_parent = n; break; } @@ -1949,11 +1965,8 @@ void __audit_inode_child(const struct inode *parent, /* is there a matching child entry? */ list_for_each_entry(n, &context->names_list, list) { /* can only match entries that have a name */ - if (!n->name || n->type != type) - continue; - - /* if we found a parent, make sure this one is a child of it */ - if (found_parent && (n->name != found_parent->name)) + if (!n->name || + (n->type != type && n->type != AUDIT_TYPE_UNKNOWN)) continue; if (!strcmp(dname, n->name->name) || @@ -1961,6 +1974,8 @@ void __audit_inode_child(const struct inode *parent, found_parent ? found_parent->name_len : AUDIT_NAME_FULL)) { + if (n->type == AUDIT_TYPE_UNKNOWN) + n->type = type; found_child = n; break; } @@ -1989,6 +2004,7 @@ void __audit_inode_child(const struct inode *parent, found_child->name_put = false; } } + if (inode) audit_copy_inode(found_child, dentry, inode); else From 55422d0bd292f5ad143cc32cb8bb8505257274c4 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 22 Jan 2015 00:00:23 -0500 Subject: [PATCH 7/7] audit: replace getname()/putname() hacks with reference counters In order to ensure that filenames are not released before the audit subsystem is done with the strings there are a number of hacks built into the fs and audit subsystems around getname() and putname(). To say these hacks are "ugly" would be kind. This patch removes the filename hackery in favor of a more conventional reference count based approach. The diffstat below tells most of the story; lots of audit/fs specific code is replaced with a traditional reference count based approach that is easily understood, even by those not familiar with the audit and/or fs subsystems. CC: viro@zeniv.linux.org.uk CC: linux-fsdevel@vger.kernel.org Signed-off-by: Paul Moore Signed-off-by: Al Viro --- fs/namei.c | 29 ++++++------ include/linux/audit.h | 3 -- include/linux/fs.h | 9 +--- kernel/audit.h | 17 +------ kernel/auditsc.c | 105 +++++------------------------------------- 5 files changed, 29 insertions(+), 134 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index a3fde77d4abf..96ca11dea4a2 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -118,15 +118,6 @@ * POSIX.1 2.4: an empty pathname is invalid (ENOENT). * PATH_MAX includes the nul terminator --RR. */ -void final_putname(struct filename *name) -{ - if (name->separate) { - __putname(name->name); - kfree(name); - } else { - __putname(name); - } -} #define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename)) @@ -145,6 +136,7 @@ getname_flags(const char __user *filename, int flags, int *empty) result = __getname(); if (unlikely(!result)) return ERR_PTR(-ENOMEM); + result->refcnt = 1; /* * First, try to embed the struct filename inside the names_cache @@ -179,6 +171,7 @@ recopy: } result->name = kname; result->separate = true; + result->refcnt = 1; max = PATH_MAX; goto recopy; } @@ -202,7 +195,7 @@ recopy: return result; error: - final_putname(result); + putname(result); return err; } @@ -243,19 +236,25 @@ getname_kernel(const char * filename) memcpy((char *)result->name, filename, len); result->uptr = NULL; result->aname = NULL; + result->refcnt = 1; audit_getname(result); return result; } -#ifdef CONFIG_AUDITSYSCALL void putname(struct filename *name) { - if (unlikely(!audit_dummy_context())) - return audit_putname(name); - final_putname(name); + BUG_ON(name->refcnt <= 0); + + if (--name->refcnt > 0) + return; + + if (name->separate) { + __putname(name->name); + kfree(name); + } else + __putname(name); } -#endif static int check_acl(struct inode *inode, int mask) { diff --git a/include/linux/audit.h b/include/linux/audit.h index af84234e1f6e..87c2d347d255 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -128,7 +128,6 @@ extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1, extern void __audit_syscall_exit(int ret_success, long ret_value); extern struct filename *__audit_reusename(const __user char *uptr); extern void __audit_getname(struct filename *name); -extern void audit_putname(struct filename *name); #define AUDIT_INODE_PARENT 1 /* dentry represents the parent */ #define AUDIT_INODE_HIDDEN 2 /* audit record should be hidden */ @@ -353,8 +352,6 @@ static inline struct filename *audit_reusename(const __user char *name) } static inline void audit_getname(struct filename *name) { } -static inline void audit_putname(struct filename *name) -{ } static inline void __audit_inode(struct filename *name, const struct dentry *dentry, unsigned int flags) diff --git a/include/linux/fs.h b/include/linux/fs.h index f90c0282c114..b49842fe203f 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2080,6 +2080,7 @@ struct filename { const char *name; /* pointer to actual string */ const __user char *uptr; /* original userland pointer */ struct audit_names *aname; + int refcnt; bool separate; /* should "name" be freed? */ }; @@ -2101,6 +2102,7 @@ extern int filp_close(struct file *, fl_owner_t id); extern struct filename *getname_flags(const char __user *, int, int *); extern struct filename *getname(const char __user *); extern struct filename *getname_kernel(const char *); +extern void putname(struct filename *name); enum { FILE_CREATED = 1, @@ -2121,15 +2123,8 @@ extern void __init vfs_caches_init(unsigned long); extern struct kmem_cache *names_cachep; -extern void final_putname(struct filename *name); - #define __getname() kmem_cache_alloc(names_cachep, GFP_KERNEL) #define __putname(name) kmem_cache_free(names_cachep, (void *)(name)) -#ifndef CONFIG_AUDITSYSCALL -#define putname(name) final_putname(name) -#else -extern void putname(struct filename *name); -#endif #ifdef CONFIG_BLOCK extern int register_blkdev(unsigned int, const char *); diff --git a/kernel/audit.h b/kernel/audit.h index 3cdffad5a1d9..1caa0d345d90 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -24,12 +24,6 @@ #include #include -/* 0 = no checking - 1 = put_count checking - 2 = verbose put_count checking -*/ -#define AUDIT_DEBUG 0 - /* AUDIT_NAMES is the number of slots we reserve in the audit_context * for saving names from getname(). If we get more names we will allocate * a name dynamically and also add those to the list anchored by names_list. */ @@ -74,9 +68,8 @@ struct audit_cap_data { }; }; -/* When fs/namei.c:getname() is called, we store the pointer in name and - * we don't let putname() free it (instead we free all of the saved - * pointers at syscall exit time). +/* When fs/namei.c:getname() is called, we store the pointer in name and bump + * the refcnt in the associated filename struct. * * Further, in fs/namei.c:path_lookup() we store the inode and device. */ @@ -86,7 +79,6 @@ struct audit_names { struct filename *name; int name_len; /* number of chars to log */ bool hidden; /* don't log this record */ - bool name_put; /* call __putname()? */ unsigned long ino; dev_t dev; @@ -208,11 +200,6 @@ struct audit_context { }; int fds[2]; struct audit_proctitle proctitle; - -#if AUDIT_DEBUG - int put_count; - int ino_count; -#endif }; extern u32 audit_ever_enabled; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 4f521964ccaa..0c38604a630c 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -866,33 +866,10 @@ static inline void audit_free_names(struct audit_context *context) { struct audit_names *n, *next; -#if AUDIT_DEBUG == 2 - if (context->put_count + context->ino_count != context->name_count) { - int i = 0; - - pr_err("%s:%d(:%d): major=%d in_syscall=%d" - " name_count=%d put_count=%d ino_count=%d" - " [NOT freeing]\n", __FILE__, __LINE__, - context->serial, context->major, context->in_syscall, - context->name_count, context->put_count, - context->ino_count); - list_for_each_entry(n, &context->names_list, list) { - pr_err("names[%d] = %p = %s\n", i++, n->name, - n->name->name ?: "(null)"); - } - dump_stack(); - return; - } -#endif -#if AUDIT_DEBUG - context->put_count = 0; - context->ino_count = 0; -#endif - list_for_each_entry_safe(n, next, &context->names_list, list) { list_del(&n->list); - if (n->name && n->name_put) - final_putname(n->name); + if (n->name) + putname(n->name); if (n->should_free) kfree(n); } @@ -1711,9 +1688,6 @@ static struct audit_names *audit_alloc_name(struct audit_context *context, list_add_tail(&aname->list, &context->names_list); context->name_count++; -#if AUDIT_DEBUG - context->ino_count++; -#endif return aname; } @@ -1734,8 +1708,10 @@ __audit_reusename(const __user char *uptr) list_for_each_entry(n, &context->names_list, list) { if (!n->name) continue; - if (n->name->uptr == uptr) + if (n->name->uptr == uptr) { + n->name->refcnt++; return n->name; + } } return NULL; } @@ -1752,19 +1728,8 @@ void __audit_getname(struct filename *name) struct audit_context *context = current->audit_context; struct audit_names *n; - if (!context->in_syscall) { -#if AUDIT_DEBUG == 2 - pr_err("%s:%d(:%d): ignoring getname(%p)\n", - __FILE__, __LINE__, context->serial, name); - dump_stack(); -#endif + if (!context->in_syscall) return; - } - -#if AUDIT_DEBUG - /* The filename _must_ have a populated ->name */ - BUG_ON(!name->name); -#endif n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN); if (!n) @@ -1772,56 +1737,13 @@ void __audit_getname(struct filename *name) n->name = name; n->name_len = AUDIT_NAME_FULL; - n->name_put = true; name->aname = n; + name->refcnt++; if (!context->pwd.dentry) get_fs_pwd(current->fs, &context->pwd); } -/* audit_putname - intercept a putname request - * @name: name to intercept and delay for putname - * - * If we have stored the name from getname in the audit context, - * then we delay the putname until syscall exit. - * Called from include/linux/fs.h:putname(). - */ -void audit_putname(struct filename *name) -{ - struct audit_context *context = current->audit_context; - - BUG_ON(!context); - if (!name->aname || !context->in_syscall) { -#if AUDIT_DEBUG == 2 - pr_err("%s:%d(:%d): final_putname(%p)\n", - __FILE__, __LINE__, context->serial, name); - if (context->name_count) { - struct audit_names *n; - int i = 0; - - list_for_each_entry(n, &context->names_list, list) - pr_err("name[%d] = %p = %s\n", i++, n->name, - n->name->name ?: "(null)"); - } -#endif - final_putname(name); - } -#if AUDIT_DEBUG - else { - ++context->put_count; - if (context->put_count > context->name_count) { - pr_err("%s:%d(:%d): major=%d in_syscall=%d putname(%p)" - " name_count=%d put_count=%d\n", - __FILE__, __LINE__, - context->serial, context->major, - context->in_syscall, name->name, - context->name_count, context->put_count); - dump_stack(); - } - } -#endif -} - /** * __audit_inode - store the inode and device from a lookup * @name: name being audited @@ -1842,11 +1764,6 @@ void __audit_inode(struct filename *name, const struct dentry *dentry, if (!name) goto out_alloc; -#if AUDIT_DEBUG - /* The struct filename _must_ have a populated ->name */ - BUG_ON(!name->name); -#endif - /* * If we have a pointer to an audit_names entry already, then we can * just use it directly if the type is correct. @@ -1893,9 +1810,10 @@ out_alloc: n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN); if (!n) return; - if (name) - /* no need to set ->name_put as the original will cleanup */ + if (name) { n->name = name; + name->refcnt++; + } out: if (parent) { @@ -2000,8 +1918,7 @@ void __audit_inode_child(const struct inode *parent, if (found_parent) { found_child->name = found_parent->name; found_child->name_len = AUDIT_NAME_FULL; - /* don't call __putname() */ - found_child->name_put = false; + found_child->name->refcnt++; } }