[PATCH] audit: fix use-after-free in audit_add_watch

* [PATCH] audit: fix use-after-free in audit_add_watch
@ 2018-06-18 13:27 Ronny Chevalier
  2018-06-18 18:35 ` Richard Guy Briggs
  0 siblings, 1 reply; 4+ messages in thread
From: Ronny Chevalier @ 2018-06-18 13:27 UTC (permalink / raw)
  To: linux-audit; +Cc: ronny.chevalier

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>
---
 kernel/audit_watch.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f1ba88994508..4bb8d7718fbc 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -421,6 +421,14 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list)
 
 	mutex_unlock(&audit_filter_mutex);
 
+	/*
+	 * 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);
+
 	/* Avoid calling path_lookup under audit_filter_mutex. */
 	ret = audit_get_nd(watch, &parent_path);
 
@@ -446,6 +454,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;
 }
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread