lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 074/111] audit: fix use-after-free in audit_add_watch
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Ronny Chevalier <ronny.chevalier@hp.com>

    [ Upstream commit baa2a4fdd525c8c4b0f704d20457195b29437839 ]

    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>
    Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
    Signed-off-by: Paul Moore <paul@paul-moore.com>
    Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    kernel/audit_watch.c | 12 +++++++++++-
    1 file changed, 11 insertions(+), 1 deletion(-)

    --- a/kernel/audit_watch.c
    +++ b/kernel/audit_watch.c
    @@ -419,6 +419,13 @@ int audit_add_watch(struct audit_krule *
    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. */
    @@ -427,8 +434,10 @@ int audit_add_watch(struct audit_krule *
    /* 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));
    @@ -446,6 +455,7 @@ int audit_add_watch(struct audit_krule *
    *list = &audit_inode_hash[h];
    error:
    path_put(&parent_path);
    + audit_put_watch(watch);
    return ret;
    }


    \
     
     \ /
      Last update: 2018-09-24 15:02    [W:4.101 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site