[PATCH] ext4: check the acl's validity before setting

[PATCH] ext4: check the acl's validity before setting

[ZhangZhen]

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(-)

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;
-- 
1.6.0.2

Re: [PATCH] ext4: check the acl's validity before setting

[Christoph Hellwig]

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.

Re: [PATCH] ext4: check the acl's validity before setting

[ZhangZhen]

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!

Re: [PATCH] ext4: check the acl's validity before setting

[Christoph Hellwig]

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.