linux-kernel - [PATCH 3.12 30/72] audit: correctly record file names with different path name types

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <8b8c7dccfc747fe8027a84eec59b4f656fa12322.1448283890.git.jslaby@suse.cz>
Date:	Mon, 23 Nov 2015 14:08:52 +0100
From:	Jiri Slaby <jslaby@...e.cz>
To:	stable@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Paul Moore <pmoore@...hat.com>,
	Jiri Slaby <jslaby@...e.cz>
Subject: [PATCH 3.12 30/72] audit: correctly record file names with different path name types

From: Paul Moore <pmoore@...hat.com>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 4a92843601ad0f5067f441d2f0dca55bbe18c076 upstream.

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@...wei.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@...wei.com>
Signed-off-by: Paul Moore <pmoore@...hat.com>
Acked-by: Richard Guy Briggs <rgb@...hat.com>
Signed-off-by: Jiri Slaby <jslaby@...e.cz>
---
 kernel/auditsc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 979c00bf24aa..24bec1d98074 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1834,12 +1834,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;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/