diff mbox

ext4: check the acl's validity before setting

Message ID 534358CA.3050808@huawei.com
State Rejected, archived
Headers show

Commit Message

ZhangZhen April 8, 2014, 2:02 a.m. UTC
Before setting the acl, call posix_acl_valid() to check if it is
valid or not.

Signed-off-by: zhang zhen <zhenzhang.zhang@huawei.com>
---
 fs/ext4/acl.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Comments

Christoph Hellwig April 8, 2014, 5:46 a.m. UTC | #1
On Tue, Apr 08, 2014 at 10:02:50AM +0800, ZhangZhen wrote:
> Before setting the acl, call posix_acl_valid() to check if it is
> valid or not.

Why? posix_acl_xattr_set already does that for you, and it's the only
way to feed in an ACL from userspace.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
ZhangZhen April 8, 2014, 8:59 a.m. UTC | #2
On 2014/4/8 13:46, Christoph Hellwig wrote:
> On Tue, Apr 08, 2014 at 10:02:50AM +0800, ZhangZhen wrote:
>> Before setting the acl, call posix_acl_valid() to check if it is
>> valid or not.
> 
> Why? posix_acl_xattr_set already does that for you, and it's the only
> way to feed in an ACL from userspace.
> 
> 
> 
Hi Hellwig,

You are right, this patch is useless. I'm sorry for this spam mail.

But btrfs_set_acl checks the validity of the ACL too. Should we delete it?
Another choice is we can check the validity in every fs tree, and delete the
check from posix_acl_xattr_set.

How should we do?

Thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig April 8, 2014, 10:40 a.m. UTC | #3
On Tue, Apr 08, 2014 at 04:59:48PM +0800, ZhangZhen wrote:
> You are right, this patch is useless. I'm sorry for this spam mail.
> 
> But btrfs_set_acl checks the validity of the ACL too. Should we delete it?
> Another choice is we can check the validity in every fs tree, and delete the
> check from posix_acl_xattr_set.

Doing it in common code is the right thing.  I suspect btrfs having it
is an oversight, but please verify it with the btrfs maintainers.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index d40c8db..97f7650 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -197,6 +197,12 @@  __ext4_set_acl(handle_t *handle, struct inode *inode, int type,
 	size_t size = 0;
 	int error;

+	if (acl) {
+		error = posix_acl_valid(acl);
+		if (error < 0)
+			return error;
+	}
+
 	switch (type) {
 	case ACL_TYPE_ACCESS:
 		name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;