linux-kernel - Re: [PATCH v18 21/22] ext4: Add richacl support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHc6FU4trGRagkjJBA9Hf_fGfCrwhRRDkUbozRxk4E1O-CbXgw@mail.gmail.com>
Date:	Mon, 14 Mar 2016 00:49:57 +0100
From:	Andreas Gruenbacher <agruenba@...hat.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
	"Theodore Ts'o" <tytso@....edu>, linux-cifs@...r.kernel.org,
	Linux API <linux-api@...r.kernel.org>,
	Trond Myklebust <trond.myklebust@...marydata.com>,
	LKML <linux-kernel@...r.kernel.org>,
	XFS Developers <xfs@....sgi.com>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Jeff Layton <jlayton@...chiereds.net>,
	linux-ext4 <linux-ext4@...r.kernel.org>,
	Anna Schumaker <anna.schumaker@...app.com>
Subject: Re: [PATCH v18 21/22] ext4: Add richacl support

On Fri, Mar 11, 2016 at 3:27 PM, Christoph Hellwig <hch@...radead.org> wrote:
>> +static int
>> +__ext4_set_richacl(handle_t *handle, struct inode *inode, struct richacl *acl)
>> +{
>> +     const int name_index = EXT4_XATTR_INDEX_RICHACL;
>> +     umode_t mode = inode->i_mode;
>> +     int retval, size;
>> +     void *value;
>> +
>> +     if (richacl_equiv_mode(acl, &mode) == 0) {
>> +             inode->i_ctime = ext4_current_time(inode);
>> +             inode->i_mode = mode;
>> +             ext4_mark_inode_dirty(handle, inode);
>> +             return __ext4_remove_richacl(handle, inode);
>> +     }
>
> Should this check for a NULL acl instead of special casing that
> in ext4_set_richacl?

I'm not sure I understand what you mean. When iop->set_richacl is
called with a richacl that is mode-equivalent, the file permission
bits need to be updated and any existing acl needs to be removed.
Doing this at the vfs level would result in two calls, iop->setattr
and iop->set_richacl, which can cause problems. To remove an existing
acl without setting the mode, set_richacl is called with a NULL
richacl.

__ext4_set_richacl() was split into __ext4_set_richacl() and
__ext4_remove_richacl() to align with the xfs code due to the
following comment from Dave Chinner:
  http://oss.sgi.com/archives/xfs/2015-10/msg00354.html

Diff here:
  https://git.kernel.org/cgit/linux/kernel/git/agruen/linux-richacl.git/diff/fs/ext4/richacl.c?id=richacl-2015-10-16&id2=richacl-2015-10-12

>> +int
>> +ext4_init_richacl(handle_t *handle, struct inode *inode, struct inode *dir)
>> +{
>> +     struct richacl *acl = richacl_create(&inode->i_mode, dir);
>> +     int error;
>> +
>> +     error = PTR_ERR(acl);
>> +     if (IS_ERR(acl))
>> +             return error;
>
>         if (IS_ERR(acl))
>                 return PTR_ERR(acl);
>
>> +     if (acl) {
>> +             error = __ext4_set_richacl(handle, inode, acl);
>> +             richacl_put(acl);
>> +     }
>
> Shouldn't richacl_create return NULL if the ACL is equivalent to the
> mode bits instead of letting every filesystem figure that out on it's
> own?

Hm, that's what it does?

Thanks,
Andreas