posix_acl: Add the check items
PATCH v2 2/12 posix_acl: Add the check items
move the ACL validation check in to fs/posix_acl.c. Including nullpointer check and PTR_ERR check.
---
---
fs/btrfs/acl.c +0 -0
@@ -102,12 +102,9 @@ static int btrfs_set_acl(struct btrfs_trans_handle
*trans,
char *value = NULL;mode_t mode;
- if (acl) {
- ret = posix_acl_valid(acl);
- if (ret < 0)
- return ret;
- ret = 0;
- }
+ ret = posix_acl_valid(acl);
+ if (ret < 0)
+ return ret;
switch (type) {
case ACL_TYPE_ACCESS:
Signature · 2 lines
Best Regards, Liuwenyi
Quoted 4 lines · Liuwenyi
(quoted diff) +0
-0@@ -102,12 +102,9 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans, char *value = NULL; mode_t mode;
Review static int btrfs_set_acl(struct btrfs_trans_handle · line 108
Quoted 10 lines
(quoted diff) +3
-6- if (acl) { - ret = posix_acl_valid(acl); - if (ret < 0) - return ret; - ret = 0; - } + ret = posix_acl_valid(acl); + if (ret < 0) + return ret;
... and now you can't remove acl from btrfs inode. At all.
NAK the entire series. You are changing existing user-visible behaviour for all filesystems for no reason and at least in this case it's clearly wrong change.
Hi All:
I'm sorry for my stupid. I am crazy for fixing this bug.
And make so much mistake.
Maybe I need cool down and think more.
--
Best Regards,
Liuwenyi
2009-12-22
2009/12/22 Al Viro <[email protected]>:
Quoted 2 lines · Liuwenyi
Review static int btrfs_set_acl(struct btrfs_trans_han=
Quoted 1 line
(quoted diff) +0
-0@@ -102,12 +102,9 @@ static int btrfs_set_acl(struct btrfs_trans_han=
dle
Quoted 14 lines
Review static int btrfs_set_acl(struct btrfs_trans_han= · line 108
Quoted 13 lines
(quoted diff) +3
-6*trans, char *value =3D NULL; mode_t mode; - if (acl) { - ret =3D posix_acl_valid(acl); - if (ret < 0) - return ret; - ret =3D 0; - } + ret =3D posix_acl_valid(acl); + if (ret < 0) + return ret;
Quoted 3 lines
... and now you can't remove acl from btrfs inode. =A0At all.
NAK the entire series. =A0You are changing existing user-visible beha=
viour
Quoted 1 line
for all filesystems for no reason and at least in this case it's clea=
rly
Quoted 2 lines
wrong change.
Quoted 4 lines · liu weni
Hi All:
I'm sorry for my stupid. I am crazy for fixing this bug.
And make so much mistake.
Welcome to the club. It's OK, _everyone_ around here had done that. Many times.
Quoted 1 line
Maybe I need cool down and think more.
Always a good advice, but in this case it's a bit more specific - if you see that you are modifying behaviour for some arguments, figure out what would lead to such arguments and see if that behaviour is intentional ;-) And if after that you are still not sure whether it's a bug or not, ask the people involved what the hell had they intended to happen in such-and-such case (often enough that'll bring variants of "nice catch" - bugs *do* happen).