lkml.org 
[lkml]   [2005]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [patch 12/13] ACL umask handling workaround in nfs client
    From
    Date
    On Tue, 2005-02-15 at 19:04, Trond Myklebust wrote:
    > lau den 22.01.2005 Klokka 21:34 (+0100) skreiv Andreas Gruenbacher:
    > > vanlig tekstdokument vedlegg (patches.suse)
    > > NFSv3 has no concept of a umask on the server side: The client applies
    > > the umask locally, and sends the effective permissions to the server.
    > > This behavior is wrong when files are created in a directory that has
    > > a default ACL. In this case, the umask is supposed to be ignored, and
    > > only the default ACL determines the file's effective permissions.
    > >
    > > Usually its the server's task to conditionally apply the umask. But
    > > since the server knows nothing about the umask, we have to do it on the
    > > client side. This patch tries to fetch the parent directory's default
    > > ACL before creating a new file, computes the appropriate create mode to
    > > send to the server, and finally sets the new file's access and default
    > > acl appropriately.
    >
    > Firstly, this sort of code belongs in the NFSv3-specific code. POSIX
    > acls have no business whatsoever in the generic NFS code.

    See attached patch.

    NOTE:

    During testing I noticed that without
    nfsacl-cache-acls-on-the-nfs-client-side.patch, no directories or
    devices can be created. It's probably a problem with
    nfs_set_default_acl(). I'll have to debug this tomorrow.

    > Secondly, what is the point of doing all this *after* you have created
    > the file with the wrong permissions? How are you avoiding races?

    Well, everything but the umask is always correct; that is guaranteed by
    the server. The initial create sets permissions that may be more
    restrictive than necessary, and then the SETACL RPC sets up the final,
    correct permissions. I don't believe that a race-free solution is
    possible.

    Cheers,
    --
    Andreas Gruenbacher <agruen@suse.de>
    SUSE Labs, SUSE LINUX GMBH
    Index: linux-2.6.11-rc3/fs/nfs/dir.c
    ===================================================================
    --- linux-2.6.11-rc3.orig/fs/nfs/dir.c
    +++ linux-2.6.11-rc3/fs/nfs/dir.c
    @@ -42,12 +42,15 @@ static int nfs_opendir(struct inode *, s
    static int nfs_readdir(struct file *, void *, filldir_t);
    static struct dentry *nfs_lookup(struct inode *, struct dentry *, struct nameidata *);
    static int nfs_create(struct inode *, struct dentry *, int, struct nameidata *);
    +static int nfs3_create(struct inode *, struct dentry *, int, struct nameidata *);
    static int nfs_mkdir(struct inode *, struct dentry *, int);
    +static int nfs3_mkdir(struct inode *, struct dentry *, int);
    static int nfs_rmdir(struct inode *, struct dentry *);
    static int nfs_unlink(struct inode *, struct dentry *);
    static int nfs_symlink(struct inode *, struct dentry *, const char *);
    static int nfs_link(struct dentry *, struct inode *, struct dentry *);
    static int nfs_mknod(struct inode *, struct dentry *, int, dev_t);
    +static int nfs3_mknod(struct inode *, struct dentry *, int, dev_t);
    static int nfs_rename(struct inode *, struct dentry *,
    struct inode *, struct dentry *);
    static int nfs_fsync_dir(struct file *, struct dentry *, int);
    @@ -77,14 +80,14 @@ struct inode_operations nfs_dir_inode_op

    #ifdef CONFIG_NFS_V3
    struct inode_operations nfs3_dir_inode_operations = {
    - .create = nfs_create,
    + .create = nfs3_create,
    .lookup = nfs_lookup,
    .link = nfs_link,
    .unlink = nfs_unlink,
    .symlink = nfs_symlink,
    - .mkdir = nfs_mkdir,
    + .mkdir = nfs3_mkdir,
    .rmdir = nfs_rmdir,
    - .mknod = nfs_mknod,
    + .mknod = nfs3_mknod,
    .rename = nfs_rename,
    .permission = nfs_permission,
    .getattr = nfs_getattr,
    @@ -994,16 +997,14 @@ out_err:
    return error;
    }

    -static int nfs_set_default_acl(struct inode *dir, struct inode *inode,
    - mode_t mode)
    +#ifdef CONFIG_NFS_V3
    +static int nfs3_set_default_acl(struct inode *dir, struct inode *inode,
    + mode_t mode)
    {
    #ifdef CONFIG_NFS_ACL
    struct posix_acl *dfacl, *acl;
    int error = 0;

    - if (NFS_PROTO(inode)->version != 3 ||
    - !NFS_PROTO(dir)->getacl || !NFS_PROTO(inode)->setacls)
    - return 0;
    dfacl = NFS_PROTO(dir)->getacl(dir, ACL_TYPE_DEFAULT);
    if (IS_ERR(dfacl)) {
    error = PTR_ERR(dfacl);
    @@ -1028,6 +1029,7 @@ out:
    return 0;
    #endif
    }
    +#endif

    /*
    * Following a failed create operation, we drop the dentry rather
    @@ -1060,7 +1062,7 @@ static int nfs_create(struct inode *dir,
    d_instantiate(dentry, inode);
    nfs_renew_times(dentry);
    nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
    - error = nfs_set_default_acl(dir, inode, mode);
    + error = 0;
    } else {
    error = PTR_ERR(inode);
    d_drop(dentry);
    @@ -1069,6 +1071,22 @@ static int nfs_create(struct inode *dir,
    return error;
    }

    +#ifdef CONFIG_NFS_V3
    +static int nfs3_create(struct inode *dir, struct dentry *dentry, int mode,
    + struct nameidata *nd)
    +{
    + int error;
    +
    + lock_kernel();
    + error = nfs_create(dir, dentry, mode, nd);
    + if (!error)
    + error = nfs3_set_default_acl(dir, dentry->d_inode, mode);
    + unlock_kernel();
    +
    + return error;
    +}
    +#endif
    +
    /*
    * See comments for nfs_proc_create regarding failed operations.
    */
    @@ -1098,9 +1116,21 @@ nfs_mknod(struct inode *dir, struct dent
    error = nfs_instantiate(dentry, &fhandle, &fattr);
    else
    d_drop(dentry);
    + unlock_kernel();
    + return error;
    +}
    +
    +static int nfs3_mknod(struct inode *dir, struct dentry *dentry, int mode,
    + dev_t rdev)
    +{
    + int error;
    +
    + lock_kernel();
    + error = nfs_mknod(dir, dentry, mode, rdev);
    if (!error)
    - error = nfs_set_default_acl(dir, dentry->d_inode, mode);
    + error = nfs3_set_default_acl(dir, dentry->d_inode, mode);
    unlock_kernel();
    +
    return error;
    }

    @@ -1138,9 +1168,20 @@ static int nfs_mkdir(struct inode *dir,
    error = nfs_instantiate(dentry, &fhandle, &fattr);
    else
    d_drop(dentry);
    + unlock_kernel();
    + return error;
    +}
    +
    +static int nfs3_mkdir(struct inode *dir, struct dentry *dentry, int mode)
    +{
    + int error;
    +
    + lock_kernel();
    + error = nfs_mkdir(dir, dentry, mode);
    if (!error)
    - error = nfs_set_default_acl(dir, dentry->d_inode, mode);
    + error = nfs3_set_default_acl(dir, dentry->d_inode, mode);
    unlock_kernel();
    +
    return error;
    }
    \
     
     \ /
      Last update: 2005-03-22 14:10    [W:4.610 / U:0.492 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site