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 051/203] nfsd: check permissions when setting ACLs
    Date
    4.6-stable review patch.  If anyone has any objections, please let me know.

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

    From: Ben Hutchings <ben@decadent.org.uk>

    commit 999653786df6954a31044528ac3f7a5dadca08f4 upstream.

    Use set_posix_acl, which includes proper permission checks, instead of
    calling ->set_acl directly. Without this anyone may be able to grant
    themselves permissions to a file by setting the ACL.

    Lock the inode to make the new checks atomic with respect to set_acl.
    (Also, nfsd was the only caller of set_acl not locking the inode, so I
    suspect this may fix other races.)

    This also simplifies the code, and ensures our ACLs are checked by
    posix_acl_valid.

    The permission checks and the inode locking were lost with commit
    4ac7249e, which changed nfsd to use the set_acl inode operation directly
    instead of going through xattr handlers.

    Reported-by: David Sinquin <david@sinquin.eu>
    [agreunba@redhat.com: use set_posix_acl]
    Fixes: 4ac7249e
    Cc: Christoph Hellwig <hch@infradead.org>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/nfsd/nfs2acl.c | 20 ++++++++++----------
    fs/nfsd/nfs3acl.c | 16 +++++++---------
    fs/nfsd/nfs4acl.c | 16 ++++++++--------
    3 files changed, 25 insertions(+), 27 deletions(-)

    --- a/fs/nfsd/nfs2acl.c
    +++ b/fs/nfsd/nfs2acl.c
    @@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct
    goto out;

    inode = d_inode(fh->fh_dentry);
    - if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
    - error = -EOPNOTSUPP;
    - goto out_errno;
    - }

    error = fh_want_write(fh);
    if (error)
    goto out_errno;

    - error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
    + fh_lock(fh);
    +
    + error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
    if (error)
    - goto out_drop_write;
    - error = inode->i_op->set_acl(inode, argp->acl_default,
    - ACL_TYPE_DEFAULT);
    + goto out_drop_lock;
    + error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
    if (error)
    - goto out_drop_write;
    + goto out_drop_lock;
    +
    + fh_unlock(fh);

    fh_drop_write(fh);

    @@ -131,7 +130,8 @@ out:
    posix_acl_release(argp->acl_access);
    posix_acl_release(argp->acl_default);
    return nfserr;
    -out_drop_write:
    +out_drop_lock:
    + fh_unlock(fh);
    fh_drop_write(fh);
    out_errno:
    nfserr = nfserrno(error);
    --- a/fs/nfsd/nfs3acl.c
    +++ b/fs/nfsd/nfs3acl.c
    @@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct s
    goto out;

    inode = d_inode(fh->fh_dentry);
    - if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
    - error = -EOPNOTSUPP;
    - goto out_errno;
    - }

    error = fh_want_write(fh);
    if (error)
    goto out_errno;

    - error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
    + fh_lock(fh);
    +
    + error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
    if (error)
    - goto out_drop_write;
    - error = inode->i_op->set_acl(inode, argp->acl_default,
    - ACL_TYPE_DEFAULT);
    + goto out_drop_lock;
    + error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);

    -out_drop_write:
    +out_drop_lock:
    + fh_unlock(fh);
    fh_drop_write(fh);
    out_errno:
    nfserr = nfserrno(error);
    --- a/fs/nfsd/nfs4acl.c
    +++ b/fs/nfsd/nfs4acl.c
    @@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
    dentry = fhp->fh_dentry;
    inode = d_inode(dentry);

    - if (!inode->i_op->set_acl || !IS_POSIXACL(inode))
    - return nfserr_attrnotsupp;
    -
    if (S_ISDIR(inode->i_mode))
    flags = NFS4_ACL_DIR;

    @@ -782,16 +779,19 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
    if (host_error < 0)
    goto out_nfserr;

    - host_error = inode->i_op->set_acl(inode, pacl, ACL_TYPE_ACCESS);
    + fh_lock(fhp);
    +
    + host_error = set_posix_acl(inode, ACL_TYPE_ACCESS, pacl);
    if (host_error < 0)
    - goto out_release;
    + goto out_drop_lock;

    if (S_ISDIR(inode->i_mode)) {
    - host_error = inode->i_op->set_acl(inode, dpacl,
    - ACL_TYPE_DEFAULT);
    + host_error = set_posix_acl(inode, ACL_TYPE_DEFAULT, dpacl);
    }

    -out_release:
    +out_drop_lock:
    + fh_unlock(fhp);
    +
    posix_acl_release(pacl);
    posix_acl_release(dpacl);
    out_nfserr:

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