* [PATCH] Btrfs: fix incorrect inode acl reset
@ 2013-10-15 17:44 Filipe David Borba Manana
2013-10-15 18:00 ` Josef Bacik
0 siblings, 1 reply; 5+ messages in thread
From: Filipe David Borba Manana @ 2013-10-15 17:44 UTC (permalink / raw)
To: linux-btrfs; +Cc: Filipe David Borba Manana
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(-)
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 {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: fix incorrect inode acl reset
2013-10-15 17:44 [PATCH] Btrfs: fix incorrect inode acl reset Filipe David Borba Manana
@ 2013-10-15 18:00 ` Josef Bacik
2013-10-15 18:02 ` Filipe David Manana
0 siblings, 1 reply; 5+ messages in thread
From: Josef Bacik @ 2013-10-15 18:00 UTC (permalink / raw)
To: Filipe David Borba Manana; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix incorrect inode acl reset
2013-10-15 18:00 ` Josef Bacik
@ 2013-10-15 18:02 ` Filipe David Manana
2013-10-15 21:58 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Filipe David Manana @ 2013-10-15 18:02 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs@vger.kernel.org
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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix incorrect inode acl reset
2013-10-15 18:02 ` Filipe David Manana
@ 2013-10-15 21:58 ` David Sterba
2013-10-16 14:06 ` Filipe David Manana
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2013-10-15 21:58 UTC (permalink / raw)
To: Filipe David Manana; +Cc: Josef Bacik, linux-btrfs@vger.kernel.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix incorrect inode acl reset
2013-10-15 21:58 ` David Sterba
@ 2013-10-16 14:06 ` Filipe David Manana
0 siblings, 0 replies; 5+ messages in thread
From: Filipe David Manana @ 2013-10-16 14:06 UTC (permalink / raw)
To: dsterba@suse.cz, Filipe David Manana, Josef Bacik,
linux-btrfs@vger.kernel.org
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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-16 14:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-15 17:44 [PATCH] Btrfs: fix incorrect inode acl reset Filipe David Borba Manana
2013-10-15 18:00 ` Josef Bacik
2013-10-15 18:02 ` Filipe David Manana
2013-10-15 21:58 ` David Sterba
2013-10-16 14:06 ` Filipe David Manana
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.