Commit eb642d00 authored by Ronny Chevalier's avatar Ronny Chevalier Committed by Kleber Sacilotto de Souza

audit: fix use-after-free in audit_add_watch

BugLink: https://bugs.launchpad.net/bugs/1798587

[ Upstream commit baa2a4fd ]

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: default avatarRonny Chevalier <ronny.chevalier@hp.com>
Reviewed-by: default avatarRichard Guy Briggs <rgb@redhat.com>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarStefan Bader <stefan.bader@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent a11f2d5b
...@@ -419,6 +419,13 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list) ...@@ -419,6 +419,13 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list)
struct path parent_path; struct path parent_path;
int h, ret = 0; 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); mutex_unlock(&audit_filter_mutex);
/* Avoid calling path_lookup under audit_filter_mutex. */ /* Avoid calling path_lookup under audit_filter_mutex. */
...@@ -427,8 +434,10 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list) ...@@ -427,8 +434,10 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list)
/* caller expects mutex locked */ /* caller expects mutex locked */
mutex_lock(&audit_filter_mutex); mutex_lock(&audit_filter_mutex);
if (ret) if (ret) {
audit_put_watch(watch);
return ret; return ret;
}
/* either find an old parent or attach a new one */ /* either find an old parent or attach a new one */
parent = audit_find_parent(d_backing_inode(parent_path.dentry)); parent = audit_find_parent(d_backing_inode(parent_path.dentry));
...@@ -446,6 +455,7 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list) ...@@ -446,6 +455,7 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list)
*list = &audit_inode_hash[h]; *list = &audit_inode_hash[h];
error: error:
path_put(&parent_path); path_put(&parent_path);
audit_put_watch(watch);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment