| Message ID | 1381859040-11211-1-git-send-email-fdmanana@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Tue, Oct 15, 2013 at 06:44:00PM +0100, Filipe David Borba Manana wrote: > When a directory has a default ACL and a subdirectory is created > under that directory, btrfs_init_acl() is called when the > subdirectory's inode is created to initialize the inode's ACL > (inherited from the parent directory) but it was clearing the ACL > from the inode after setting it if posix_acl_create() returned > success, instead of clearing it only if it returned an error. > > To reproduce this issue: > > $ mkfs.btrfs -f /dev/loop0 > $ mount /dev/loop0 /mnt > $ mkdir /mnt/acl > $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl > $ getfacl /mnt/acl > user::rwx > group::rwx > other::r-x > default:user::rwx > default:group::rwx > default:other::--- > > $ mkdir /mnt/acl/dir1 > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > > After unmounting and mounting again the filesystem, fgetacl returned the > expected ACL: > > $ umount /mnt/acl > $ mount /dev/loop0 /mnt > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > default:user::rwx > default:group::rwx > default:other::--- > > Meaning that the underlying xattr was persisted. > Can you put this into an xfstest please? Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 15, 2013 at 7:00 PM, Josef Bacik <jbacik@fusionio.com> wrote: > On Tue, Oct 15, 2013 at 06:44:00PM +0100, Filipe David Borba Manana wrote: >> When a directory has a default ACL and a subdirectory is created >> under that directory, btrfs_init_acl() is called when the >> subdirectory's inode is created to initialize the inode's ACL >> (inherited from the parent directory) but it was clearing the ACL >> from the inode after setting it if posix_acl_create() returned >> success, instead of clearing it only if it returned an error. >> >> To reproduce this issue: >> >> $ mkfs.btrfs -f /dev/loop0 >> $ mount /dev/loop0 /mnt >> $ mkdir /mnt/acl >> $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl >> $ getfacl /mnt/acl >> user::rwx >> group::rwx >> other::r-x >> default:user::rwx >> default:group::rwx >> default:other::--- >> >> $ mkdir /mnt/acl/dir1 >> $ getfacl /mnt/acl/dir1 >> user::rwx >> group::rwx >> other::--- >> >> After unmounting and mounting again the filesystem, fgetacl returned the >> expected ACL: >> >> $ umount /mnt/acl >> $ mount /dev/loop0 /mnt >> $ getfacl /mnt/acl/dir1 >> user::rwx >> group::rwx >> other::--- >> default:user::rwx >> default:group::rwx >> default:other::--- >> >> Meaning that the underlying xattr was persisted. >> > > Can you put this into an xfstest please? Thanks, A new one or just add it to an existing one (and which)? > > Josef
On Tue, Oct 15, 2013 at 07:02:47PM +0100, Filipe David Manana wrote: > > Can you put this into an xfstest please? Thanks, > > A new one or just add it to an existing one (and which)? There's xfstests/shared/051 that tests ACLs, but is run only against xfs and udf. I haven't verified it wrt btrfs, but maybe this test catches the bug. If not, please add a new test into the 'generic' category. thanks, david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 15, 2013 at 10:58 PM, David Sterba <dsterba@suse.cz> wrote: > On Tue, Oct 15, 2013 at 07:02:47PM +0100, Filipe David Manana wrote: >> > Can you put this into an xfstest please? Thanks, >> >> A new one or just add it to an existing one (and which)? > > There's xfstests/shared/051 that tests ACLs, but is run only against xfs > and udf. I haven't verified it wrt btrfs, but maybe this test catches > the bug. If not, please add a new test into the 'generic' category. I adapted shared/051 to make it work with btrfs too. However the test as it is, doesn't trigger the default ACL inheritance issue, so I added the steps from the commit message to the test. Thanks David. > > thanks, > david
diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c index e15d2b0..0890c83 100644 --- a/fs/btrfs/acl.c +++ b/fs/btrfs/acl.c @@ -229,7 +229,7 @@ int btrfs_init_acl(struct btrfs_trans_handle *trans, if (ret > 0) { /* we need an acl */ ret = btrfs_set_acl(trans, inode, acl, ACL_TYPE_ACCESS); - } else { + } else if (ret < 0) { cache_no_acl(inode); } } else {
When a directory has a default ACL and a subdirectory is created under that directory, btrfs_init_acl() is called when the subdirectory's inode is created to initialize the inode's ACL (inherited from the parent directory) but it was clearing the ACL from the inode after setting it if posix_acl_create() returned success, instead of clearing it only if it returned an error. To reproduce this issue: $ mkfs.btrfs -f /dev/loop0 $ mount /dev/loop0 /mnt $ mkdir /mnt/acl $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl $ getfacl /mnt/acl user::rwx group::rwx other::r-x default:user::rwx default:group::rwx default:other::--- $ mkdir /mnt/acl/dir1 $ getfacl /mnt/acl/dir1 user::rwx group::rwx other::--- After unmounting and mounting again the filesystem, fgetacl returned the expected ACL: $ umount /mnt/acl $ mount /dev/loop0 /mnt $ getfacl /mnt/acl/dir1 user::rwx group::rwx other::--- default:user::rwx default:group::rwx default:other::--- Meaning that the underlying xattr was persisted. Reported-by: Giuseppe Fierro <giuseppe@fierro.org> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> --- fs/btrfs/acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)