Re: [PATCH v2] 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; --- v2: - Move audit_get_watch before audit_find_parent. In the case of audit_get_nd failing. --- kernel/audit_watch.c | 9 +++++++++ 1 file changed, 9 insertions(+)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index f1ba88994508..6d9b3f2bb1e2 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -430,6 +430,14 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list) if (ret) return ret; + /* + * 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); + /* either find an old parent or attach a new one */ parent = audit_find_parent(d_backing_inode(parent_path.dentry)); if (!parent) { @@ -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