lkml.org 
[lkml]   [2016]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.6 092/203] apparmor: fix oops, validate buffer size in apparmor_setprocattr()
    Date
    4.6-stable review patch.  If anyone has any objections, please let me know.

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

    From: Vegard Nossum <vegard.nossum@oracle.com>

    commit 30a46a4647fd1df9cf52e43bf467f0d9265096ca upstream.

    When proc_pid_attr_write() was changed to use memdup_user apparmor's
    (interface violating) assumption that the setprocattr buffer was always
    a single page was violated.

    The size test is not strictly speaking needed as proc_pid_attr_write()
    will reject anything larger, but for the sake of robustness we can keep
    it in.

    SMACK and SELinux look safe to me, but somebody else should probably
    have a look just in case.

    Based on original patch from Vegard Nossum <vegard.nossum@oracle.com>
    modified for the case that apparmor provides null termination.

    Fixes: bb646cdb12e75d82258c2f2e7746d5952d3e321a
    Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: John Johansen <john.johansen@canonical.com>
    Cc: Paul Moore <paul@paul-moore.com>
    Cc: Stephen Smalley <sds@tycho.nsa.gov>
    Cc: Eric Paris <eparis@parisplace.org>
    Cc: Casey Schaufler <casey@schaufler-ca.com>
    Signed-off-by: John Johansen <john.johansen@canonical.com>
    Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
    Signed-off-by: James Morris <james.l.morris@oracle.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    security/apparmor/lsm.c | 36 +++++++++++++++++++-----------------
    1 file changed, 19 insertions(+), 17 deletions(-)

    --- a/security/apparmor/lsm.c
    +++ b/security/apparmor/lsm.c
    @@ -523,34 +523,34 @@ static int apparmor_setprocattr(struct t
    {
    struct common_audit_data sa;
    struct apparmor_audit_data aad = {0,};
    - char *command, *args = value;
    + char *command, *largs = NULL, *args = value;
    size_t arg_size;
    int error;

    if (size == 0)
    return -EINVAL;
    - /* args points to a PAGE_SIZE buffer, AppArmor requires that
    - * the buffer must be null terminated or have size <= PAGE_SIZE -1
    - * so that AppArmor can null terminate them
    - */
    - if (args[size - 1] != '\0') {
    - if (size == PAGE_SIZE)
    - return -EINVAL;
    - args[size] = '\0';
    - }
    -
    /* task can only write its own attributes */
    if (current != task)
    return -EACCES;

    - args = value;
    + /* AppArmor requires that the buffer must be null terminated atm */
    + if (args[size - 1] != '\0') {
    + /* null terminate */
    + largs = args = kmalloc(size + 1, GFP_KERNEL);
    + if (!args)
    + return -ENOMEM;
    + memcpy(args, value, size);
    + args[size] = '\0';
    + }
    +
    + error = -EINVAL;
    args = strim(args);
    command = strsep(&args, " ");
    if (!args)
    - return -EINVAL;
    + goto out;
    args = skip_spaces(args);
    if (!*args)
    - return -EINVAL;
    + goto out;

    arg_size = size - (args - (char *) value);
    if (strcmp(name, "current") == 0) {
    @@ -576,10 +576,12 @@ static int apparmor_setprocattr(struct t
    goto fail;
    } else
    /* only support the "current" and "exec" process attributes */
    - return -EINVAL;
    + goto fail;

    if (!error)
    error = size;
    +out:
    + kfree(largs);
    return error;

    fail:
    @@ -588,9 +590,9 @@ fail:
    aad.profile = aa_current_profile();
    aad.op = OP_SETPROCATTR;
    aad.info = name;
    - aad.error = -EINVAL;
    + aad.error = error = -EINVAL;
    aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
    - return -EINVAL;
    + goto out;
    }

    static int apparmor_task_setrlimit(struct task_struct *task,

    \
     
     \ /
      Last update: 2016-07-26 00:01    [W:4.140 / U:1.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site