[Acked] [Trusty][Utopic][SRU][PATCH 1/1] audit: correctly record file names with different path name types

Andy Whitcroft apw at canonical.com
Thu Apr 2 12:06:36 UTC 2015


On Thu, Apr 02, 2015 at 07:05:21PM +0800, Gavin Guo wrote:
> From: Paul Moore <pmoore at redhat.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1439441
> 
> 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
> 
> 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>
> (cherry picked from commit 4a92843601ad0f5067f441d2f0dca55bbe18c076)
> Signed-off-by: Gavin Guo <gavin.guo at canonical.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;

Looks to do what is claimed.  Represents a regression against P.

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw




More information about the kernel-team mailing list