Skip to content

Commit

Permalink
audit: fix use-after-free in audit_add_watch
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Richard Guy Briggs <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
ronnychevalier authored and pcmoore committed Jul 18, 2018
1 parent 290e44b commit baa2a4f
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion kernel/audit_watch.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,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. */
Expand All @@ -428,8 +435,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(d_backing_inode(parent_path.dentry));
Expand All @@ -447,6 +456,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;
}

Expand Down

0 comments on commit baa2a4f

Please sign in to comment.