APPLIED: [Trusty][SRU][PATCH] audit: create private file name copies when auditing inodes

[Brad Figg]

On Thu, Apr 30, 2015 at 11:56:52PM -0500, Chris J Arges wrote:
> From: Paul Moore <pmoore at redhat.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1450442
> 
> Unfortunately, while commit 4a928436 ("audit: correctly record file
> names with different path name types") fixed a problem where we were
> not recording filenames, it created a new problem by attempting to use
> these file names after they had been freed.  This patch resolves the
> issue by creating a copy of the filename which the audit subsystem
> frees after it is done with the string.
> 
> At some point it would be nice to resolve this issue with refcounts,
> or something similar, instead of having to allocate/copy strings, but
> that is almost surely beyond the scope of a -rcX patch so we'll defer
> that for later.  On the plus side, only audit users should be impacted
> by the string copying.
> 
> Reported-by: Toralf Foerster <toralf.foerster at gmx.de>
> Signed-off-by: Paul Moore <pmoore at redhat.com>
> (cherry picked from commit fcf22d8267ad2601fe9b6c549d1be96401c23e0b)
> Signed-off-by: Chris J Arges <chris.j.arges at canonical.com>
> 
> Conflicts:
> 	kernel/auditsc.c
> ---
>  kernel/auditsc.c | 51 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4c6ffa0..b3be41e 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -68,6 +68,8 @@
>  #include <linux/capability.h>
>  #include <linux/fs_struct.h>
>  #include <linux/compat.h>
> +#include <linux/string.h>
> +#include <uapi/linux/limits.h>
>  
>  #include "audit.h"
>  
> @@ -1801,8 +1803,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
>  	}
>  
>  	list_for_each_entry_reverse(n, &context->names_list, list) {
> -		/* does the name pointer match? */
> -		if (!n->name || n->name->name != name->name)
> +		if (!n->name || strcmp(n->name->name, name->name))
>  			continue;
>  
>  		/* match the correct record type */
> @@ -1821,14 +1822,44 @@ out_alloc:
>  	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
>  	if (!n)
>  		return;
> -	if (name)
> -		/* since name is not NULL we know there is already a matching
> -		 * name record, see audit_getname(), so there must be a type
> -		 * mismatch; reuse the string path since the original name
> -		 * record will keep the string valid until we free it in
> -		 * audit_free_names() */
> -		n->name = name;
> -
> +	/* 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 (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;
> -- 
> 1.9.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Applied to Trusty master-next.

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com