2014-04-08 02:04:01

by Zhang Zhen

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

Before setting the acl, call posix_acl_valid() to check if it is
valid or not.

Signed-off-by: zhang zhen <[email protected]>
---
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






2014-04-08 05:46:40

by Christoph Hellwig

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

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.


2014-04-08 09:04:38

by Zhang Zhen

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

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!



2014-04-08 10:40:32

by Christoph Hellwig

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

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.