lkml.org 
[lkml]   [2015]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.4 009/172] smack: fix possible use after frees in task_security() callers
    Date
    From: Andrey Ryabinin <a.ryabinin@samsung.com>

    3.4.108-rc1 review patch. If anyone has any objections, please let me know.

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


    commit 6d1cff2a885850b78b40c34777b46cf5da5d1050 upstream.

    We hit use after free on dereferncing pointer to task_smack struct in
    smk_of_task() called from smack_task_to_inode().

    task_security() macro uses task_cred_xxx() to get pointer to the task_smack.
    task_cred_xxx() could be used only for non-pointer members of task's
    credentials. It cannot be used for pointer members since what they point
    to may disapper after dropping RCU read lock.

    Mainly task_security() used this way:
    smk_of_task(task_security(p))

    Intead of this introduce function smk_of_task_struct() which
    takes task_struct as argument and returns pointer to smk_known struct
    and do this under RCU read lock.
    Bogus task_security() macro is not used anymore, so remove it.

    KASan's report for this:

    AddressSanitizer: use after free in smack_task_to_inode+0x50/0x70 at addr c4635600
    =============================================================================
    BUG kmalloc-64 (Tainted: PO): kasan error
    -----------------------------------------------------------------------------

    Disabling lock debugging due to kernel taint
    INFO: Allocated in new_task_smack+0x44/0xd8 age=39 cpu=0 pid=1866
    kmem_cache_alloc_trace+0x88/0x1bc
    new_task_smack+0x44/0xd8
    smack_cred_prepare+0x48/0x21c
    security_prepare_creds+0x44/0x4c
    prepare_creds+0xdc/0x110
    smack_setprocattr+0x104/0x150
    security_setprocattr+0x4c/0x54
    proc_pid_attr_write+0x12c/0x194
    vfs_write+0x1b0/0x370
    SyS_write+0x5c/0x94
    ret_fast_syscall+0x0/0x48
    INFO: Freed in smack_cred_free+0xc4/0xd0 age=27 cpu=0 pid=1564
    kfree+0x270/0x290
    smack_cred_free+0xc4/0xd0
    security_cred_free+0x34/0x3c
    put_cred_rcu+0x58/0xcc
    rcu_process_callbacks+0x738/0x998
    __do_softirq+0x264/0x4cc
    do_softirq+0x94/0xf4
    irq_exit+0xbc/0x120
    handle_IRQ+0x104/0x134
    gic_handle_irq+0x70/0xac
    __irq_svc+0x44/0x78
    _raw_spin_unlock+0x18/0x48
    sync_inodes_sb+0x17c/0x1d8
    sync_filesystem+0xac/0xfc
    vdfs_file_fsync+0x90/0xc0
    vfs_fsync_range+0x74/0x7c
    INFO: Slab 0xd3b23f50 objects=32 used=31 fp=0xc4635600 flags=0x4080
    INFO: Object 0xc4635600 @offset=5632 fp=0x (null)

    Bytes b4 c46355f0: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
    Object c4635600: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
    Object c4635610: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
    Object c4635620: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
    Object c4635630: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
    Redzone c4635640: bb bb bb bb ....
    Padding c46356e8: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
    Padding c46356f8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
    CPU: 5 PID: 834 Comm: launchpad_prelo Tainted: PBO 3.10.30 #1
    Backtrace:
    [<c00233a4>] (dump_backtrace+0x0/0x158) from [<c0023dec>] (show_stack+0x20/0x24)
    r7:c4634010 r6:d3b23f50 r5:c4635600 r4:d1002140
    [<c0023dcc>] (show_stack+0x0/0x24) from [<c06d6d7c>] (dump_stack+0x20/0x28)
    [<c06d6d5c>] (dump_stack+0x0/0x28) from [<c01c1d50>] (print_trailer+0x124/0x144)
    [<c01c1c2c>] (print_trailer+0x0/0x144) from [<c01c1e88>] (object_err+0x3c/0x44)
    r7:c4635600 r6:d1002140 r5:d3b23f50 r4:c4635600
    [<c01c1e4c>] (object_err+0x0/0x44) from [<c01cac18>] (kasan_report_error+0x2b8/0x538)
    r6:d1002140 r5:d3b23f50 r4:c6429cf8 r3:c09e1aa7
    [<c01ca960>] (kasan_report_error+0x0/0x538) from [<c01c9430>] (__asan_load4+0xd4/0xf8)
    [<c01c935c>] (__asan_load4+0x0/0xf8) from [<c031e168>] (smack_task_to_inode+0x50/0x70)
    r5:c4635600 r4:ca9da000
    [<c031e118>] (smack_task_to_inode+0x0/0x70) from [<c031af64>] (security_task_to_inode+0x3c/0x44)
    r5:cca25e80 r4:c0ba9780
    [<c031af28>] (security_task_to_inode+0x0/0x44) from [<c023d614>] (pid_revalidate+0x124/0x178)
    r6:00000000 r5:cca25e80 r4:cbabe3c0 r3:00008124
    [<c023d4f0>] (pid_revalidate+0x0/0x178) from [<c01db98c>] (lookup_fast+0x35c/0x43y4)
    r9:c6429efc r8:00000101 r7:c079d940 r6:c6429e90 r5:c6429ed8 r4:c83c4148
    [<c01db630>] (lookup_fast+0x0/0x434) from [<c01deec8>] (do_last.isra.24+0x1c0/0x1108)
    [<c01ded08>] (do_last.isra.24+0x0/0x1108) from [<c01dff04>] (path_openat.isra.25+0xf4/0x648)
    [<c01dfe10>] (path_openat.isra.25+0x0/0x648) from [<c01e1458>] (do_filp_open+0x3c/0x88)
    [<c01e141c>] (do_filp_open+0x0/0x88) from [<c01ccb28>] (do_sys_open+0xf0/0x198)
    r7:00000001 r6:c0ea2180 r5:0000000b r4:00000000
    [<c01cca38>] (do_sys_open+0x0/0x198) from [<c01ccc00>] (SyS_open+0x30/0x34)
    [<c01ccbd0>] (SyS_open+0x0/0x34) from [<c001db80>] (ret_fast_syscall+0x0/0x48)
    Read of size 4 by thread T834:
    Memory state around the buggy address:
    c4635380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    c4635400: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
    c4635480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    c4635500: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
    c4635580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    >c4635600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ^
    c4635680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    c4635700: 00 00 00 00 04 fc fc fc fc fc fc fc fc fc fc fc
    c4635780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    c4635800: 00 00 00 00 00 00 04 fc fc fc fc fc fc fc fc fc
    c4635880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    ==================================================================

    Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
    [lizf: Backported to 3.4:
    - smk_of_task() returns char* instead of smack_known *
    - replace task_security() with smk_of_task() with smk_of_task_struct()
    manually]
    Signed-off-by: Zefan Li <lizefan@huawei.com>
    ---
    security/smack/smack.h | 10 ++++++++++
    security/smack/smack_lsm.c | 24 +++++++++++++-----------
    2 files changed, 23 insertions(+), 11 deletions(-)

    diff --git a/security/smack/smack.h b/security/smack/smack.h
    index 4ede719..cde4cc1 100644
    --- a/security/smack/smack.h
    +++ b/security/smack/smack.h
    @@ -298,6 +298,16 @@ static inline char *smk_of_current(void)
    return smk_of_task(current_security());
    }

    +static inline char *smk_of_task_struct(const struct task_struct *t)
    +{
    + char *skp;
    +
    + rcu_read_lock();
    + skp = smk_of_task(__task_cred(t)->security);
    + rcu_read_unlock();
    + return skp;
    +}
    +
    /*
    * logging functions
    */
    diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
    index 45c32f0..82df24d 100644
    --- a/security/smack/smack_lsm.c
    +++ b/security/smack/smack_lsm.c
    @@ -41,8 +41,6 @@
    #include <linux/binfmts.h>
    #include "smack.h"

    -#define task_security(task) (task_cred_xxx((task), security))
    -
    #define TRANS_TRUE "TRUE"
    #define TRANS_TRUE_SIZE 4

    @@ -164,7 +162,7 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
    if (rc != 0)
    return rc;

    - tsp = smk_of_task(task_security(ctp));
    + tsp = smk_of_task_struct(ctp);
    smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
    smk_ad_setfield_u_tsk(&ad, ctp);

    @@ -190,7 +188,7 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
    if (rc != 0)
    return rc;

    - tsp = smk_of_task(task_security(ptp));
    + tsp = smk_of_task_struct(ptp);
    smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
    smk_ad_setfield_u_tsk(&ad, ptp);

    @@ -1518,7 +1516,7 @@ static int smk_curacc_on_task(struct task_struct *p, int access,

    smk_ad_init(&ad, caller, LSM_AUDIT_DATA_TASK);
    smk_ad_setfield_u_tsk(&ad, p);
    - return smk_curacc(smk_of_task(task_security(p)), access, &ad);
    + return smk_curacc(smk_of_task_struct(p), access, &ad);
    }

    /**
    @@ -1564,7 +1562,7 @@ static int smack_task_getsid(struct task_struct *p)
    */
    static void smack_task_getsecid(struct task_struct *p, u32 *secid)
    {
    - *secid = smack_to_secid(smk_of_task(task_security(p)));
    + *secid = smack_to_secid(smk_of_task_struct(p));
    }

    /**
    @@ -1676,7 +1674,7 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info,
    * can write the receiver.
    */
    if (secid == 0)
    - return smk_curacc(smk_of_task(task_security(p)), MAY_WRITE,
    + return smk_curacc(smk_of_task_struct(p), MAY_WRITE,
    &ad);
    /*
    * If the secid isn't 0 we're dealing with some USB IO
    @@ -1684,7 +1682,7 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info,
    * we can't take privilege into account.
    */
    return smk_access(smack_from_secid(secid),
    - smk_of_task(task_security(p)), MAY_WRITE, &ad);
    + smk_of_task_struct(p), MAY_WRITE, &ad);
    }

    /**
    @@ -1697,9 +1695,13 @@ static int smack_task_wait(struct task_struct *p)
    {
    struct smk_audit_info ad;
    char *sp = smk_of_current();
    - char *tsp = smk_of_forked(task_security(p));
    + char *tsp;
    int rc;

    + rcu_read_lock();
    + tsp = smk_of_forked(__task_cred(p)->security);
    + rcu_read_unlock();
    +
    /* we don't log here, we can be overriden */
    rc = smk_access(tsp, sp, MAY_WRITE, NULL);
    if (rc == 0)
    @@ -1736,7 +1738,7 @@ static int smack_task_wait(struct task_struct *p)
    static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
    {
    struct inode_smack *isp = inode->i_security;
    - isp->smk_inode = smk_of_task(task_security(p));
    + isp->smk_inode = smk_of_task_struct(p);
    }

    /*
    @@ -2719,7 +2721,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
    if (strcmp(name, "current") != 0)
    return -EINVAL;

    - cp = kstrdup(smk_of_task(task_security(p)), GFP_KERNEL);
    + cp = kstrdup(smk_of_task_struct(p), GFP_KERNEL);
    if (cp == NULL)
    return -ENOMEM;

    --
    1.9.1


    \
     
     \ /
      Last update: 2015-06-16 12:01    [W:4.211 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site