@@ -1503,7 +1503,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
if (open_flags & O_CREAT) {
attr.ia_valid |= ATTR_MODE;
- attr.ia_mode = mode & ~current_umask();
+ attr.ia_mode = mode;
}
if (open_flags & O_TRUNC) {
attr.ia_valid |= ATTR_SIZE;
@@ -2793,6 +2793,81 @@ out:
return status;
}
+static bool nfs4_ace_inheritable(u32 flags)
+{
+ return flags & (NFS4_ACE_FILE_INHERIT_ACE |
+ NFS4_ACE_DIRECTORY_INHERIT_ACE);
+
+}
+
+/* XXX: one-off by-hand xdr parsing: */
+static int nfs4_acl_has_inheritable_aces(void *buf, int len)
+{
+ __be32 *p = buf;
+ int naces, i;
+ u32 word;
+
+ if (len < 4)
+ goto overflow;
+ len -= 4;
+ naces = be32_to_cpup(p++);
+ for (i=0; i < naces; i++) {
+ if (len < 4*4)
+ goto overflow;
+ len -= 4 * 4;
+ p++; /* skip type */
+ word = be32_to_cpup(p++); /* flags */
+ if (nfs4_ace_inheritable(word))
+ return 1;
+ p++; /* skip access mask; */
+ /* skip name: */
+ word = be32_to_cpup(p++);
+ if (len < (XDR_QUADLEN(word) << 2))
+ goto overflow;
+ len -= XDR_QUADLEN(word) << 2;
+ p += XDR_QUADLEN(word);
+ }
+ return 0;
+overflow:
+ /* XXX: what should we do with too-long ACLs? */
+ return -EIO;
+}
+
+static ssize_t nfs4_proc_get_acl(struct inode *inode, void *buf, size_t buflen);
+
+/*
+ * Apply the umask iff the directory lacks inheritable ACLs.
+ *
+ * XXX: Callers are ignoring the error cases.
+ */
+static int nfs4_apply_umask(struct inode *dir, unsigned short *mode)
+{
+ struct page *page;
+ void *buf;
+ int ret;
+
+ /* XXX: We probably need to handle longer ACLs: */
+ page = alloc_page(GFP_KERNEL);
+ if (!page)
+ return -ENOMEM;
+ buf = page_address(page);
+
+ ret = nfs4_proc_get_acl(dir, buf, PAGE_SIZE);
+ if (ret < 0)
+ goto out_free;
+ ret = nfs4_acl_has_inheritable_aces(buf, PAGE_SIZE);
+ if (ret < 0)
+ goto out_free;
+ if (ret) {
+ ret = 0;
+ goto out_free;
+ }
+ *mode &= ~current_umask();
+out_free:
+ __free_page(page);
+ return ret;
+}
+
static struct inode *
nfs4_atomic_open(struct inode *dir, struct nfs_open_context *ctx,
int open_flags, struct iattr *attr, int *opened)
@@ -2800,6 +2875,9 @@ nfs4_atomic_open(struct inode *dir, struct nfs_open_context *ctx,
struct nfs4_state *state;
struct nfs4_label l = {0, 0, 0, NULL}, *label = NULL;
+ if (open_flags & O_CREAT)
+ nfs4_apply_umask(dir, &attr->ia_mode);
+
label = nfs4_label_init_security(dir, ctx->dentry, attr, &l);
/* Protect against concurrent sillydeletes */
@@ -3507,7 +3585,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
- sattr->ia_mode &= ~current_umask();
+ nfs4_apply_umask(dir, &sattr->ia_mode);
state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, &opened);
if (IS_ERR(state)) {
status = PTR_ERR(state);
@@ -3818,7 +3896,7 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
label = nfs4_label_init_security(dir, dentry, sattr, &l);
- sattr->ia_mode &= ~current_umask();
+ nfs4_apply_umask(dir, &sattr->ia_mode);
do {
err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
trace_nfs4_mkdir(dir, &dentry->d_name, err);
@@ -3927,7 +4005,7 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
label = nfs4_label_init_security(dir, dentry, sattr, &l);
- sattr->ia_mode &= ~current_umask();
+ nfs4_apply_umask(dir, &sattr->ia_mode);
do {
err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
trace_nfs4_mknod(dir, &dentry->d_name, err);
We've gotten complaints that the effect of NFSv4 ACL inheritance is often overridden by common umask settings. Options: - Do nothing, make sure the behavior's documented. I don't think this will make people happy, but I haven't tried to figure out exactly what any of them are trying to do. - Do as in NFSv3 and teach the client to skip the umask in the case the parent directory has inheritable ACEs. The following patch is a proof-of-concept with some bugs. It requires documenting what we mean by "parent directory has interitable ACEs"--I'm taking it to mean that the ACL has some ACE with inheritance bits set, as that's simple to explain. Drawbacks include: requires fetching and caching an ACL on create; there's a race between checking and creating; creates may fail just because reading the ACL fails. - New protocol: e.g. add a new attribute representing the un-umasked mode, send both that and the regular mode on create, let the server sort it out. - Something else??: adopt some convention that uses redundant information in a compound (multiple SETATTRs?) to communicate umask to "in the know" servers without changing behavior on existing servers. Provide an "ignore the umask" mount option. Maybe I'm missing a clever trick. Any ideas? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html