On Sat, Sep 24, 2022 at 07:22:50PM +0100, Al Viro wrote:
> On Thu, Sep 22, 2022 at 05:17:05PM +0200, Christian Brauner wrote:
>
> > +int v9fs_iop_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
> > + struct posix_acl *acl, int type)
> > +{
> > + int retval;
> > + void *value = NULL;
> > + size_t size = 0;
> > + struct v9fs_session_info *v9ses;
> > + struct inode *inode = d_inode(dentry);
> > +
> > + v9ses = v9fs_dentry2v9ses(dentry);
> > +
> > + if (acl) {
> > + retval = posix_acl_valid(inode->i_sb->s_user_ns, acl);
> > + if (retval)
> > + goto err_out;
> > +
> > + size = posix_acl_xattr_size(acl->a_count);
> > +
> > + value = kzalloc(size, GFP_NOFS);
> > + if (!value) {
> > + retval = -ENOMEM;
> > + goto err_out;
> > + }
> > +
> > + retval = posix_acl_to_xattr(&init_user_ns, acl, value, size);
> > + if (retval < 0)
> > + goto err_out;
> > + }
> > +
> > + /*
> > + * set the attribute on the remote. Without even looking at the
> > + * xattr value. We leave it to the server to validate
> > + */
> > + if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
> > + retval = v9fs_xattr_set(dentry, posix_acl_xattr_name(type),
> > + value, size, 0);
> > + goto err_out;
> > + }
>
> > + if (S_ISLNK(inode->i_mode))
> > + return -EOPNOTSUPP;
> > + if (!inode_owner_or_capable(&init_user_ns, inode))
> > + return -EPERM;
>
> Shouldn't that chunk have been in the very beginning? As it is, you've
> got a leak here...
Good catch, I probably messed up the merge conflict resolution in my
last rebase... Thanks for spotting!
