[RFC PATCH] audit: correctly record file names with different path name types

[Richard Guy Briggs]

On 14/12/01, Paul Moore wrote:
> There is a problem with the audit system when multiple audit records
> are created for the same path, each with a different path name type.
> The root cause of the problem is in __audit_inode() when an exact
> match (both the path name and path name type) is not found for a
> path name record; the existing code creates a new path name record,
> but it never sets the path name in this record, leaving it NULL.
> This patch corrects this problem by assigning the path name to these
> newly created records.
> 
> There are many ways to reproduce this problem, but one of the
> easiest is the following (assuming auditd is running):
> 
>   # mkdir /root/tmp/test
>   # touch /root/tmp/test/567
>   # auditctl -a always,exit -F dir=/root/tmp/test
>   # touch /root/tmp/test/567
> 
> Afterwards, or while the commands above are running, check the audit
> log and pay special attention to the PATH records.  A faulty kernel
> will display something like the following for the file creation:
> 
>   type=SYSCALL msg=audit(1416957442.025:93): arch=c000003e syscall=2
>     success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
>   type=CWD msg=audit(1416957442.025:93):  cwd="/root/tmp"
>   type=PATH msg=audit(1416957442.025:93): item=0 name="test/"
>     inode=401409 ... nametype=PARENT
>   type=PATH msg=audit(1416957442.025:93): item=1 name=(null)
>     inode=393804 ... nametype=NORMAL
>   type=PATH msg=audit(1416957442.025:93): item=2 name=(null)
>     inode=393804 ... nametype=NORMAL
> 
> While a patched kernel will show the following:
> 
>   type=SYSCALL msg=audit(1416955786.566:89): arch=c000003e syscall=2
>     success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
>   type=CWD msg=audit(1416955786.566:89):  cwd="/root/tmp"
>   type=PATH msg=audit(1416955786.566:89): item=0 name="test/"
>     inode=401409 ... nametype=PARENT
>   type=PATH msg=audit(1416955786.566:89): item=1 name="test/567"
>     inode=393804 ... nametype=NORMAL
> 
> This issue was brought up by a number of people, but special credit
> should go to hujianyang at huawei.com for reporting the problem along
> with an explanation of the problem and a patch.  While the original
> patch did have some problems (see the archive link below), it did
> demonstrate the problem and helped kickstart the fix presented here.
> 
>   * https://lkml.org/lkml/2014/9/5/66
> 
> Cc: stable at vger.kernel.org
> Reported-by: hujianyang <hujianyang at huawei.com>
> Signed-off-by: Paul Moore <pmoore at redhat.com>

Acked-by: Richard Guy Briggs <rgb at redhat.com>

> ---
>  kernel/auditsc.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 21eae3c..ff99c05 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1886,12 +1886,18 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
>  	}
>  
>  out_alloc:
> -	/* unable to find the name from a previous getname(). Allocate a new
> -	 * anonymous entry.
> -	 */
> -	n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
> +	/* unable to find an entry with both a matching name and type */
> +	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;
> +
>  out:
>  	if (parent) {
>  		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;

- RGB

--
Richard Guy Briggs <rbriggs at redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545