lkml.org 
[lkml]   [2013]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)
    Date
    There have long been two ways to ask the kernel to create a new
    hardlink to the inode represented by an fd: linkat(..., AT_EMPTY_PATH)
    and AT_SYMLINK_FOLLOW on /proc/self/fd/N. The latter has no
    particular security restrictions, but the former required privilege
    until:

    commit bb2314b47996491bbc5add73633905c3120b6268
    Author: Andy Lutomirski <luto@amacapital.net>
    Date: Thu Aug 1 21:44:31 2013 -0700

    fs: Allow unprivileged linkat(..., AT_EMPTY_PATH) aka flink

    Spender pointed out that there could be code that passes an fd to an
    untrusted, chrooted process, and allowing that process to flink the
    fd could be dangerous. (I'm not aware of any actual known attack.)

    So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH)
    if the target is I_LINKABLE. I_LINKABLE is only set for inodes
    created by O_TMPFILE without O_EXCL, which is an explicit request
    for flinkability.

    Reported-by: Brad Spengler <spender@grsecurity.net>
    Signed-off-by: Andy Lutomirski <luto@amacapital.net>
    ---

    Changes from v1: Removed an unnecessary spin_lock.

    fs/namei.c | 30 ++++++++++++++++++++++++++----
    1 file changed, 26 insertions(+), 4 deletions(-)

    diff --git a/fs/namei.c b/fs/namei.c
    index 89a612e..9802d51 100644
    --- a/fs/namei.c
    +++ b/fs/namei.c
    @@ -3652,6 +3652,26 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
    }

    /*
    + * flink is dangerous. For now, only allow it when specifically requested
    + * or when the caller is privileged.
    + *
    + * NB: This is not currently enforced for AT_SYMLINK_FOLLOW on procfs.
    + * Fixing that would be intrusive and could break something.
    + */
    +static bool may_flink(const struct path *path)
    +{
    + struct inode *inode = path->dentry->d_inode;
    +
    + /*
    + * This is racy: I_LINKABLE could be cleared between this check
    + * and the actual link operation. This is odd but not a security
    + * problem: the caller could get the same effect by flinking once
    + * and then using normal link(2) to create a second link.
    + */
    + return (inode->i_state & I_LINKABLE) || capable(CAP_DAC_READ_SEARCH);
    +}
    +
    +/*
    * Hardlinks are often used in delicate situations. We avoid
    * security-related surprises by not following symlinks on the
    * newname. --KAB
    @@ -3670,10 +3690,7 @@ SYSCALL_DEFINE5(linkat, int, olddfd, const char __user *, oldname,

    if ((flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH)) != 0)
    return -EINVAL;
    - /*
    - * Using empty names is equivalent to using AT_SYMLINK_FOLLOW
    - * on /proc/self/fd/<fd>.
    - */
    +
    if (flags & AT_EMPTY_PATH)
    how = LOOKUP_EMPTY;

    @@ -3684,6 +3701,11 @@ retry:
    if (error)
    return error;

    + if ((flags & AT_EMPTY_PATH) && !may_flink(&old_path)) {
    + error = -EPERM;
    + goto out;
    + }
    +
    new_dentry = user_path_create(newdfd, newname, &new_path,
    (how & LOOKUP_REVAL));
    error = PTR_ERR(new_dentry);
    --
    1.8.3.1


    \
     
     \ /
      Last update: 2013-08-21 21:21    [W:0.027 / U:148.324 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site