On Sat, Sep 05, 2015 at 12:27:09PM +0200, Andreas Gruenbacher wrote:
> When a new file is created, it can inherit an acl from its parent
> directory; this is similar to how default acls work in POSIX (draft)
> ACLs.
> 
> As with POSIX ACLs, if a file inherits an acl from its parent directory,
> the intersection between the create mode and the permissions granted by
> the inherited acl determines the file masks and file permission bits,
> and the umask is ignored.
> 
> Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>
> ---
>  fs/richacl_base.c       | 69 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/richacl_inode.c      | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/richacl.h |  2 ++
>  3 files changed, 136 insertions(+)
> 
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> index 106e988..fda407d 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -483,3 +483,72 @@ richacl_equiv_mode(const struct richacl *acl, mode_t 
> *mode_p)
>       return 0;
>  }
>  EXPORT_SYMBOL_GPL(richacl_equiv_mode);
> +
> +/**
> + * richacl_inherit  -  compute the inherited acl of a new file
> + * @dir_acl: acl of the containing directory
> + * @isdir:   inherit by a directory or non-directory?
> + *
> + * A directory can have acl entries which files and/or directories created
> + * inside the directory will inherit.  This function computes the acl for 
> such
> + * a new file.  If there is no inheritable acl, it will return %NULL.
> + */
> +struct richacl *
> +richacl_inherit(const struct richacl *dir_acl, int isdir)
> +{
> +     const struct richace *dir_ace;
> +     struct richacl *acl = NULL;
> +     struct richace *ace;
> +     int count = 0;
> +
> +     if (isdir) {
> +             richacl_for_each_entry(dir_ace, dir_acl) {
> +                     if (!richace_is_inheritable(dir_ace))
> +                             continue;
> +                     count++;
> +             }
> +             if (!count)
> +                     return NULL;
> +             acl = richacl_alloc(count, GFP_KERNEL);
> +             if (!acl)
> +                     return ERR_PTR(-ENOMEM);
> +             ace = acl->a_entries;
> +             richacl_for_each_entry(dir_ace, dir_acl) {
> +                     if (!richace_is_inheritable(dir_ace))
> +                             continue;
> +                     richace_copy(ace, dir_ace);
> +                     if (dir_ace->e_flags & RICHACE_NO_PROPAGATE_INHERIT_ACE)
> +                             ace->e_flags &= ~RICHACE_INHERITANCE_FLAGS;
> +                     else if ((dir_ace->e_flags & RICHACE_FILE_INHERIT_ACE) 
> &&
> +                              !(dir_ace->e_flags & 
> RICHACE_DIRECTORY_INHERIT_ACE))

The FILE_INHERIT_ACE check there is redundant since we already know
dir_ace is inheritable.

(So, OK, it isn't wrong to check it again but let's not make this
condition any more complicated than necessary.)

> +                             ace->e_flags |= RICHACE_INHERIT_ONLY_ACE;
> +                     ace++;
> +             }
> +     } else {
> +             richacl_for_each_entry(dir_ace, dir_acl) {
> +                     if (!(dir_ace->e_flags & RICHACE_FILE_INHERIT_ACE))
> +                             continue;
> +                     count++;
> +             }
> +             if (!count)
> +                     return NULL;
> +             acl = richacl_alloc(count, GFP_KERNEL);
> +             if (!acl)
> +                     return ERR_PTR(-ENOMEM);
> +             ace = acl->a_entries;
> +             richacl_for_each_entry(dir_ace, dir_acl) {
> +                     if (!(dir_ace->e_flags & RICHACE_FILE_INHERIT_ACE))
> +                             continue;
> +                     richace_copy(ace, dir_ace);
> +                     ace->e_flags &= ~RICHACE_INHERITANCE_FLAGS;
> +                     /*
> +                      * RICHACE_DELETE_CHILD is meaningless for
> +                      * non-directories, so clear it.
> +                      */
> +                     ace->e_mask &= ~RICHACE_DELETE_CHILD;
> +                     ace++;
> +             }
> +     }
> +
> +     return acl;
> +}
> diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
> index dc2a69f..f3f1f84 100644
> --- a/fs/richacl_inode.c
> +++ b/fs/richacl_inode.c
> @@ -220,3 +220,68 @@ out:
>       return denied ? -EACCES : 0;
>  }
>  EXPORT_SYMBOL_GPL(richacl_permission);
> +
> +/**
> + * richacl_inherit_inode  -  compute inherited acl and file mode
> + * @dir_acl: acl of the containing directory
> + * @inode:   inode of the new file (create mode in i_mode)
> + *
> + * The file permission bits in inode->i_mode must be set to the create mode 
> by
> + * the caller.
> + *
> + * If there is an inheritable acl, the maximum permissions that the acl 
> grants
> + * will be computed and permissions not granted by the acl will be removed 
> from
> + * inode->i_mode.  If there is no inheritable acl, the umask will be applied
> + * instead.
> + */
> +static struct richacl *
> +richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
> +{
> +     struct richacl *acl;
> +     mode_t mask;
> +
> +     acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
> +     if (acl) {
> +             mask = inode->i_mode;
> +             if (richacl_equiv_mode(acl, &mask) == 0) {
> +                     richacl_put(acl);
> +                     acl = NULL;

Why is it correct to ignore entirely the inherited acl in this case?

Oh, I see, I'm forgetting that richacl_equiv_mode is setting the mask,
which will get applied at the end of this function.  In my defense,
maybe it's easy to overlook a side effect in an if condition.... But I
don't have a better idea.  OK.

So, nits aside:

        Reviewed-by: J. Bruce Fields <bfie...@redhat.com>

--b.

> +             } else {
> +                     richacl_compute_max_masks(acl);
> +                     /*
> +                      * Ensure that the acl will not grant any permissions
> +                      * beyond the create mode.
> +                      */
> +                     acl->a_flags |= RICHACL_MASKED;
> +                     acl->a_owner_mask &=
> +                             richacl_mode_to_mask(inode->i_mode >> 6);
> +                     acl->a_group_mask &=
> +                             richacl_mode_to_mask(inode->i_mode >> 3);
> +                     acl->a_other_mask &=
> +                             richacl_mode_to_mask(inode->i_mode);
> +                     mask = ~S_IRWXUGO | richacl_masks_to_mode(acl);
> +             }
> +     } else
> +             mask = ~current_umask();
> +
> +     inode->i_mode &= mask;
> +     return acl;
> +}
> +
> +struct richacl *richacl_create(struct inode *inode, struct inode *dir)
> +{
> +     struct richacl *dir_acl, *acl = NULL;
> +
> +     if (S_ISLNK(inode->i_mode))
> +             return NULL;
> +     dir_acl = get_richacl(dir);
> +     if (dir_acl) {
> +             if (IS_ERR(dir_acl))
> +                     return dir_acl;
> +             acl = richacl_inherit_inode(dir_acl, inode);
> +             richacl_put(dir_acl);
> +     } else
> +             inode->i_mode &= ~current_umask();
> +     return acl;
> +}
> +EXPORT_SYMBOL_GPL(richacl_create);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 6535ce5..9bf95c2 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -305,8 +305,10 @@ extern unsigned int richacl_want_to_mask(unsigned int);
>  extern void richacl_compute_max_masks(struct richacl *);
>  extern struct richacl *richacl_chmod(struct richacl *, mode_t);
>  extern int richacl_equiv_mode(const struct richacl *, mode_t *);
> +extern struct richacl *richacl_inherit(const struct richacl *, int);
>  
>  /* richacl_inode.c */
>  extern int richacl_permission(struct inode *, const struct richacl *, int);
> +extern struct richacl *richacl_create(struct inode *, struct inode *);
>  
>  #endif /* __RICHACL_H */
> -- 
> 2.4.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to