Commit 2e866a27 authored Jul 11, 2018 by Ronny Chevalier Committed by Greg Kroah-Hartman Sep 26, 2018

audit: fix use-after-free in audit_add_watch



[ Upstream commit baa2a4fdd525c8c4b0f704d20457195b29437839 ]

audit_add_watch stores locally krule->watch without taking a reference
on watch. Then, it calls audit_add_to_parent, and uses the watch stored
locally.

Unfortunately, it is possible that audit_add_to_parent updates
krule->watch.
When it happens, it also drops a reference of watch which
could free the watch.

How to reproduce (with KASAN enabled):

    auditctl -w /etc/passwd -F success=0 -k test_passwd
    auditctl -w /etc/passwd -F success=1 -k test_passwd2

The second call to auditctl triggers the use-after-free, because
audit_to_parent updates krule->watch to use a previous existing watch
and drops the reference to the newly created watch.

To fix the issue, we grab a reference of watch and we release it at the
end of the function.

Signed-off-by: Ronny Chevalier <ronny.chevalier@hp.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

parent 1a92b397

kernel/audit_watch.c

+11 −1

Original line number	Diff line number	Diff line
		@@ -414,6 +414,13 @@ int audit_add_watch(struct audit_krule krule, struct list_head *list)
		struct path parent_path;
		int h, ret = 0;

		/*
		* When we will be calling audit_add_to_parent, krule->watch might have
		* been updated and watch might have been freed.
		* So we need to keep a reference of watch.
		*/
		audit_get_watch(watch);

		mutex_unlock(&audit_filter_mutex);

		/* Avoid calling path_lookup under audit_filter_mutex. */
		@@ -422,8 +429,10 @@ int audit_add_watch(struct audit_krule krule, struct list_head *list)
		/* caller expects mutex locked */
		mutex_lock(&audit_filter_mutex);

		if (ret)
		if (ret) {
		audit_put_watch(watch);
		return ret;
		}

		/* either find an old parent or attach a new one */
		parent = audit_find_parent(parent_path.dentry->d_inode);
		@@ -444,6 +453,7 @@ int audit_add_watch(struct audit_krule krule, struct list_head *list)
		*list = &audit_inode_hash[h];
		error:
		path_put(&parent_path);
		audit_put_watch(watch);
		return ret;
		}