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

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(a)hp.com&gt; --- 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; }