[Added a few more CCs]
On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
<[email protected]> wrote:
> Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to fail with EOPNOTSUPP.
> For example, having the following folders:
>
> |- nfs <- NFSv4 is mounted here
> |--|- folder
> |- root <- ext4 is mounted here
> |- work <- also ext4
> |- merged <- overlay is mounted here with
> lowerdir=nfs,upperdir=root,workdir=work
>
> And calling
> # touch merged/folder/file
> will print
> touch: cannot touch 'merged/folder/file': Operation not supported
>
> This is because NFS returns the xattr system.nfs4_acl with an empty value even if no NFS ACLs are in use in the lower filesystem. Trying to set this xattr in the upperdir
> fails because ext4 does not support it.
>
> Fix this by explicitly checking for the name of the xattr and an empty value and ignoring EOPNOTSUPP if both things match.
>
> Signed-off-by: Patrick Plagwitz <[email protected]>
> ---
> Maybe NFS could be changed to not return empty system.nfs4_acl values, I don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, returning the error code from
> vfs_setxattr with this xattr name must be avoided as long as the value is empty.
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 36795ee..505b86e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
> continue; /* Discard */
> }
> error = vfs_setxattr(new, name, value, size, 0);
> + if (error == -EOPNOTSUPP && *value == '\0' &&
> + strcmp(name, "system.nfs4_acl") == 0)
> + error = 0;
> if (error)
> break;
> }
I agree that this should be fixed, but adding such exceptions for
certain filesystems or xattrs is not the proper way, IMO.
Can NFS people comment on this? Where does the nfs4_acl come from?
What can overlayfs do if it's a non-empty ACL?
Does knfsd translate posix ACL into NFS acl? If so, we can translate
back. Should we do a generic POSIX<->NFS acl translator?
Thanks,
Miklos
On Mon, Dec 05, 2016 at 10:28:18AM +0100, Miklos Szeredi wrote:
> [Added a few more CCs]
>
> On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
> <[email protected]> wrote:
> > Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to fail with EOPNOTSUPP.
> > For example, having the following folders:
> >
> > |- nfs <- NFSv4 is mounted here
> > |--|- folder
> > |- root <- ext4 is mounted here
> > |- work <- also ext4
> > |- merged <- overlay is mounted here with
> > lowerdir=nfs,upperdir=root,workdir=work
> >
> > And calling
> > # touch merged/folder/file
> > will print
> > touch: cannot touch 'merged/folder/file': Operation not supported
> >
> > This is because NFS returns the xattr system.nfs4_acl with an empty value even if no NFS ACLs are in use in the lower filesystem. Trying to set this xattr in the upperdir
> > fails because ext4 does not support it.
> >
> > Fix this by explicitly checking for the name of the xattr and an empty value and ignoring EOPNOTSUPP if both things match.
> >
> > Signed-off-by: Patrick Plagwitz <[email protected]>
> > ---
> > Maybe NFS could be changed to not return empty system.nfs4_acl values, I don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, returning the error code from
> > vfs_setxattr with this xattr name must be avoided as long as the value is empty.
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 36795ee..505b86e 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
> > continue; /* Discard */
> > }
> > error = vfs_setxattr(new, name, value, size, 0);
> > + if (error == -EOPNOTSUPP && *value == '\0' &&
> > + strcmp(name, "system.nfs4_acl") == 0)
> > + error = 0;
> > if (error)
> > break;
> > }
>
> I agree that this should be fixed, but adding such exceptions for
> certain filesystems or xattrs is not the proper way, IMO.
>
> Can NFS people comment on this? Where does the nfs4_acl come from?
This is the interface the NFS client provides for applications to modify
NFSv4 ACLs on servers that support them.
> What can overlayfs do if it's a non-empty ACL?
As little as possible. You can't copy it up, can you? So any attempt
to support it is going to be incomplete.
> Does knfsd translate posix ACL into NFS acl? If so, we can translate
> back. Should we do a generic POSIX<->NFS acl translator?
knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
algorithm, and lossy (in the NFSv4->POSIX direction). The client
developers have been understandably reluctant to have anything to do
with it.
So, I think listxattr should omit system.nfs4_acl, and attempts to
set/get the attribute should error out. The same should apply to any
"system." attribute not supported by both filesystems, I think?
I don't understand overlayfs very well, though.
--b.
On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>> Can NFS people comment on this? Where does the nfs4_acl come from?
>
> This is the interface the NFS client provides for applications to modify
> NFSv4 ACLs on servers that support them.
Fine, but why are we seeing this xattr on exports where no xattrs are
set on the exported fs?
>> What can overlayfs do if it's a non-empty ACL?
>
> As little as possible. You can't copy it up, can you? So any attempt
> to support it is going to be incomplete.
Right.
>
>> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>> back. Should we do a generic POSIX<->NFS acl translator?
>
> knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
This does explain the nfs4_acl xattr on the client. Question: if it's
empty, why have it at all?
> algorithm, and lossy (in the NFSv4->POSIX direction). The client
> developers have been understandably reluctant to have anything to do
> with it.
>
> So, I think listxattr should omit system.nfs4_acl, and attempts to
> set/get the attribute should error out. The same should apply to any
> "system." attribute not supported by both filesystems, I think?
Basically that's what happens now. The problem is that nfsv4 mounts
seem always have these xattrs, even when the exported fs doesn't have
anything.
We could do the copy up even if the NFS4->POSIX translation was
possible (which is the case with POSIX ACL translated by knfsd). We'd
just get back the original ACL, so that's OK. NFS is only supported
as lower (read-only) layer, so we don't care about changing the ACL on
the server.
Thanks,
Miklos
On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
> >> Can NFS people comment on this? Where does the nfs4_acl come from?
> >
> > This is the interface the NFS client provides for applications to modify
> > NFSv4 ACLs on servers that support them.
>
> Fine, but why are we seeing this xattr on exports where no xattrs are
> set on the exported fs?
I don't know. I took another look at the original patch and don't see
any details on the server setup: which server is it (knfsd, ganesha,
netapp, ...)? How is it configured?
> >> What can overlayfs do if it's a non-empty ACL?
> >
> > As little as possible. You can't copy it up, can you? So any attempt
> > to support it is going to be incomplete.
>
> Right.
>
> >
> >> Does knfsd translate posix ACL into NFS acl? If so, we can translate
> >> back. Should we do a generic POSIX<->NFS acl translator?
> >
> > knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>
> This does explain the nfs4_acl xattr on the client. Question: if it's
> empty, why have it at all?
I'm honestly not sure what's going on there. I'd be curious to see a
network trace if possible.
> > algorithm, and lossy (in the NFSv4->POSIX direction). The client
> > developers have been understandably reluctant to have anything to do
> > with it.
> >
> > So, I think listxattr should omit system.nfs4_acl, and attempts to
> > set/get the attribute should error out. The same should apply to any
> > "system." attribute not supported by both filesystems, I think?
>
> Basically that's what happens now. The problem is that nfsv4 mounts
> seem always have these xattrs, even when the exported fs doesn't have
> anything.
I said "both", that's a logical "and". Whether or not nfs claims
support would then be irrelevant in this case, since ext4 doesn't
support system.nfs4_acl.
> We could do the copy up even if the NFS4->POSIX translation was
> possible (which is the case with POSIX ACL translated by knfsd). We'd
> just get back the original ACL, so that's OK.
Note that knfsd is an exception, most NFSv4-acl-supporting servers
aren't translating from POSIX ACLs.
> NFS is only supported as lower (read-only) layer, so we don't care
> about changing the ACL on the server.
Out of curiosity, how do you check permissions after copy up?
The client doesn't do much permissions-checking normally, because it's
hard to get right--even in the absence of ACLs, it may not understand
the server's owners and groups completely.
I guess that's fine, you may be happy to let people write to the file
without permissions to the lower file, since the writes aren't going
there anyway.
So, I don't know what want here.
You're not going to want to use the ACL for actual permission-checking,
and you can't modify it, so it doesn't seem very useful.
--b.
On 12/05/2016 05:25 PM, J. Bruce Fields wrote:
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>>>> Can NFS people comment on this? Where does the nfs4_acl come from?
>>>
>>> This is the interface the NFS client provides for applications to modify
>>> NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
>
> I don't know. I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)? How is it configured?
>
The server is the one in the kernel (knfsd, I assume), with completely default
configuration. /etc/exports is
/srv/nfsv4 localhost(fsid=root)
Adding rw, or various other options, does not change the observations in the
original mail. As far as I know, there are no options for controlling ACLs.
>>>> What can overlayfs do if it's a non-empty ACL?
>>>
>>> As little as possible. You can't copy it up, can you? So any attempt
>>> to support it is going to be incomplete.
>>
>> Right.
>>
>>>
>>>> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>>>> back. Should we do a generic POSIX<->NFS acl translator?
>>>
>>> knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client. Question: if it's
>> empty, why have it at all?
>
> I'm honestly not sure what's going on there. I'd be curious to see a
> network trace if possible.
>
Just make folders like in the original mail, start the NFS server with the
exports from above and touch merged/folder/file to reproduce the problem.
I don't know anything about the NFS protocol but here you have a tshark
packet summary as well as the details from the folder/ lookup reply (51).
As far as I can tell, no ACL information is in there.
22 NFS 286 V4 Call CREATE_SESSION
23 NFS 214 V4 Reply (Call In 22) CREATE_SESSION
24 NFS 214 V4 Call RECLAIM_COMPLETE
26 NFS 178 V4 Reply (Call In 24) RECLAIM_COMPLETE
27 NFS 218 V4 Call SECINFO_NO_NAME
29 NFS 194 V4 Reply (Call In 27) SECINFO_NO_NAME
30 NFS 230 V4 Call PUTROOTFH | GETATTR
31 NFS 350 V4 Reply (Call In 30) PUTROOTFH | GETATTR
32 NFS 242 V4 Call GETATTR FH: 0x62d40c52
33 NFS 254 V4 Reply (Call In 32) GETATTR
34 NFS 242 V4 Call GETATTR FH: 0x62d40c52
35 NFS 254 V4 Reply (Call In 34) GETATTR
36 NFS 242 V4 Call GETATTR FH: 0x62d40c52
37 NFS 254 V4 Reply (Call In 36) GETATTR
38 NFS 242 V4 Call GETATTR FH: 0x62d40c52
39 NFS 254 V4 Reply (Call In 38) GETATTR
40 NFS 234 V4 Call GETATTR FH: 0x62d40c52
41 NFS 206 V4 Reply (Call In 40) GETATTR
42 NFS 242 V4 Call GETATTR FH: 0x62d40c52
43 NFS 254 V4 Reply (Call In 42) GETATTR
44 NFS 238 V4 Call GETATTR FH: 0x62d40c52
45 NFS 330 V4 Reply (Call In 44) GETATTR
46 NFS 246 V4 Call ACCESS FH: 0x62d40c52, [Check: RD LU MD XT DL]
47 NFS 258 V4 Reply (Call In 46) ACCESS, [Access Denied: MD XT DL], [Allowed: RD LU]
48 NFS 238 V4 Call GETATTR FH: 0x62d40c52
49 NFS 250 V4 Reply (Call In 48) GETATTR
50 NFS 258 V4 Call LOOKUP DH: 0x62d40c52/folder
51 NFS 366 V4 Reply (Call In 50) LOOKUP
52 NFS 254 V4 Call ACCESS FH: 0x96d0c04a, [Check: RD LU MD XT DL]
53 NFS 258 V4 Reply (Call In 52) ACCESS, [Access Denied: MD XT DL], [Allowed: RD LU]
54 NFS 262 V4 Call LOOKUP DH: 0x96d0c04a/file
55 NFS 186 V4 Reply (Call In 54) LOOKUP Status: NFS4ERR_NOENT
56 NFS 246 V4 Call GETATTR FH: 0x96d0c04a
57 NFS 278 V4 Reply (Call In 56) GETATTR
Network File System, Ops(5): SEQUENCE PUTFH LOOKUP GETFH GETATTR
[Program Version: 4]
[V4 Procedure: COMPOUND (1)]
Status: NFS4_OK (0)
Tag: <EMPTY>
length: 0
contents: <EMPTY>
Operations (count: 5)
Opcode: SEQUENCE (53)
Status: NFS4_OK (0)
sessionid: 50a245584a819c9a0a00000000000000
seqid: 0x0000000d
slot id: 0
high slot id: 30
target high slot id: 30
status flags: 0x00000000
.... .... .... .... .... .... .... ...0 = SEQ4_STATUS_CB_PATH_DOWN: Not set
.... .... .... .... .... .... .... ..0. = SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRING: Not set
.... .... .... .... .... .... .... .0.. = SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRED: Not set
.... .... .... .... .... .... .... 0... = SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED: Not set
.... .... .... .... .... .... ...0 .... = SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED: Not set
.... .... .... .... .... .... ..0. .... = SEQ4_STATUS_ADMIN_STATE_REVOKED: Not set
.... .... .... .... .... .... .0.. .... = SEQ4_STATUS_RECALLABLE_STATE_REVOKED: Not set
.... .... .... .... .... .... 0... .... = SEQ4_STATUS_LEASE_MOVED: Not set
.... .... .... .... .... ...0 .... .... = SEQ4_STATUS_RESTART_RECLAIM_NEEDED: Not set
.... .... .... .... .... ..0. .... .... = SEQ4_STATUS_CB_PATH_DOWN_SESSION: Not set
.... .... .... .... .... .0.. .... .... = SEQ4_STATUS_BACKCHANNEL_FAULT: Not set
.... .... .... .... .... 0... .... .... = SEQ4_STATUS_DEVID_CHANGED: Not set
.... .... .... .... ...0 .... .... .... = SEQ4_STATUS_DEVID_DELETED: Not set
Opcode: PUTFH (22)
Status: NFS4_OK (0)
Opcode: LOOKUP (15)
Status: NFS4_OK (0)
Opcode: GETFH (10)
Status: NFS4_OK (0)
Filehandle
length: 16
[hash (CRC-32): 0x96d0c04a]
filehandle: 0100010100000000790f04000762a435
Opcode: GETATTR (9)
Status: NFS4_OK (0)
Attr mask[0]: 0x0010011a (Type, Change, Size, FSID, FileId)
reqd_attr: Type (1)
ftype4: NF4DIR (2)
reqd_attr: Change (3)
changeid: 6360241138682687914
reqd_attr: Size (4)
size: 4096
reqd_attr: FSID (8)
fattr4_fsid
fsid4.major: 0
fsid4.minor: 0
reco_attr: FileId (20)
fileid: 266105
Attr mask[1]: 0x00b0a23a (Mode, NumLinks, Owner, Owner_Group, RawDev, Space_Used, Time_Access, Time_Metadata, Time_Modify, Mounted_on_FileId)
reco_attr: Mode (33)
mode: 0755, Name: Unknown, Read permission for owner, Write permission for owner, Execute permission for owner, Read permission for group, Execute
permission for group, Read permission for others, Execute permission for others
.... .... .... .... 000. .... .... .... = Name: Unknown (0)
.... .... .... .... .... 0... .... .... = Set user id on exec: No
.... .... .... .... .... .0.. .... .... = Set group id on exec: No
.... .... .... .... .... ..0. .... .... = Save swapped text even after use: No
.... .... .... .... .... ...1 .... .... = Read permission for owner: Yes
.... .... .... .... .... .... 1... .... = Write permission for owner: Yes
.... .... .... .... .... .... .1.. .... = Execute permission for owner: Yes
.... .... .... .... .... .... ..1. .... = Read permission for group: Yes
.... .... .... .... .... .... ...0 .... = Write permission for group: No
.... .... .... .... .... .... .... 1... = Execute permission for group: Yes
.... .... .... .... .... .... .... .1.. = Read permission for others: Yes
.... .... .... .... .... .... .... ..0. = Write permission for others: No
.... .... .... .... .... .... .... ...1 = Execute permission for others: Yes
reco_attr: NumLinks (35)
numlinks: 2
reco_attr: Owner (36)
fattr4_owner: 0
length: 1
contents: 0
fill bytes: opaque data
reco_attr: Owner_Group (37)
fattr4_owner_group: 0
length: 1
contents: 0
fill bytes: opaque data
reco_attr: RawDev (41)
specdata1: 0
specdata2: 0
reco_attr: Space_Used (45)
space_used: 4096
reco_attr: Time_Access (47)
seconds: 1480888617
nseconds: 513333330
reco_attr: Time_Metadata (52)
seconds: 1480859038
nseconds: 486666666
reco_attr: Time_Modify (53)
seconds: 1480859038
nseconds: 486666666
reco_attr: Mounted_on_FileId (55)
fileid: 0x0000000000040f79
[Main Opcode: LOOKUP (15)]
2016-12-05 17:25 GMT+01:00 J. Bruce Fields <[email protected]>:
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>> >> Can NFS people comment on this? Where does the nfs4_acl come from?
>> >
>> > This is the interface the NFS client provides for applications to modify
>> > NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
>
> I don't know. I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)? How is it configured?
>
>> >> What can overlayfs do if it's a non-empty ACL?
>> >
>> > As little as possible. You can't copy it up, can you? So any attempt
>> > to support it is going to be incomplete.
>>
>> Right.
>>
>> >
>> >> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>> >> back. Should we do a generic POSIX<->NFS acl translator?
>> >
>> > knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client. Question: if it's
>> empty, why have it at all?
>
> I'm honestly not sure what's going on there. I'd be curious to see a
> network trace if possible.
I do see "system.nfs4_acl" attributes on knfsd exported filesystems
that support POSIX ACLs (for ext4: "mount -o acl"). For exported
filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
attribute is missing. The attribute shouldn't be empty though; when
the file has no real ACL, "system.nfs4_acl" represents the file mode
permissions. The "system.nfs4_acl" attribute exposes the information
on the wire; there is no resonable way to translate that into an ACL
on another filesystem, really.
Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>> > algorithm, and lossy (in the NFSv4->POSIX direction). The client
>> > developers have been understandably reluctant to have anything to do
>> > with it.
>> >
>> > So, I think listxattr should omit system.nfs4_acl, and attempts to
>> > set/get the attribute should error out. The same should apply to any
>> > "system." attribute not supported by both filesystems, I think?
>>
>> Basically that's what happens now. The problem is that nfsv4 mounts
>> seem always have these xattrs, even when the exported fs doesn't have
>> anything.
>
> I said "both", that's a logical "and". Whether or not nfs claims
> support would then be irrelevant in this case, since ext4 doesn't
> support system.nfs4_acl.
>
>> We could do the copy up even if the NFS4->POSIX translation was
>> possible (which is the case with POSIX ACL translated by knfsd). We'd
>> just get back the original ACL, so that's OK.
>
> Note that knfsd is an exception, most NFSv4-acl-supporting servers
> aren't translating from POSIX ACLs.
>
>> NFS is only supported as lower (read-only) layer, so we don't care
>> about changing the ACL on the server.
>
> Out of curiosity, how do you check permissions after copy up?
>
> The client doesn't do much permissions-checking normally, because it's
> hard to get right--even in the absence of ACLs, it may not understand
> the server's owners and groups completely.
>
> I guess that's fine, you may be happy to let people write to the file
> without permissions to the lower file, since the writes aren't going
> there anyway.
>
> So, I don't know what want here.
>
> You're not going to want to use the ACL for actual permission-checking,
> and you can't modify it, so it doesn't seem very useful.
IMO the only thing overlayfs can do is hide those attributes from the
user and ignore them when copying up. Still, the attributes shouldn't
be empty.
Andreas
On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <[email protected]>:
>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>>>>> Can NFS people comment on this? Where does the nfs4_acl come from?
>>>>
>>>> This is the interface the NFS client provides for applications to modify
>>>> NFSv4 ACLs on servers that support them.
>>>
>>> Fine, but why are we seeing this xattr on exports where no xattrs are
>>> set on the exported fs?
>>
>> I don't know. I took another look at the original patch and don't see
>> any details on the server setup: which server is it (knfsd, ganesha,
>> netapp, ...)? How is it configured?
>>
>>>>> What can overlayfs do if it's a non-empty ACL?
>>>>
>>>> As little as possible. You can't copy it up, can you? So any attempt
>>>> to support it is going to be incomplete.
>>>
>>> Right.
>>>
>>>>
>>>>> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>>>>> back. Should we do a generic POSIX<->NFS acl translator?
>>>>
>>>> knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>>>
>>> This does explain the nfs4_acl xattr on the client. Question: if it's
>>> empty, why have it at all?
>>
>> I'm honestly not sure what's going on there. I'd be curious to see a
>> network trace if possible.
>
> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
> attribute is missing. The attribute shouldn't be empty though; when
> the file has no real ACL, "system.nfs4_acl" represents the file mode
> permissions. The "system.nfs4_acl" attribute exposes the information
> on the wire; there is no resonable way to translate that into an ACL
> on another filesystem, really.
>
> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>
getfattr -m - -d nfs/folder -e text gives
# file: nfs/folder/
system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A<E7>\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000
EVERYONE@\000\000"
Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
empty after all and checking *value == 0 does not tell if there are actually
ACLs present or not, sorry for the confusion.
You are right, when I mount the exported fs with noacl the problem goes away.
You already helped me there, thanks.
Still, I think there should be a way to copy up files that actually have no
ACLs since acl is often the default for ext4 mounts and giving an "Operation
not supported" for random open(2)s is not a very good way to convey what's
going on.
2016-12-05 23:58 GMT+01:00 Patrick Plagwitz <[email protected]>:
> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <[email protected]>:
>>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>>>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>>>>>> Can NFS people comment on this? Where does the nfs4_acl come from?
>>>>>
>>>>> This is the interface the NFS client provides for applications to modify
>>>>> NFSv4 ACLs on servers that support them.
>>>>
>>>> Fine, but why are we seeing this xattr on exports where no xattrs are
>>>> set on the exported fs?
>>>
>>> I don't know. I took another look at the original patch and don't see
>>> any details on the server setup: which server is it (knfsd, ganesha,
>>> netapp, ...)? How is it configured?
>>>
>>>>>> What can overlayfs do if it's a non-empty ACL?
>>>>>
>>>>> As little as possible. You can't copy it up, can you? So any attempt
>>>>> to support it is going to be incomplete.
>>>>
>>>> Right.
>>>>
>>>>>
>>>>>> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>>>>>> back. Should we do a generic POSIX<->NFS acl translator?
>>>>>
>>>>> knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>>>>
>>>> This does explain the nfs4_acl xattr on the client. Question: if it's
>>>> empty, why have it at all?
>>>
>>> I'm honestly not sure what's going on there. I'd be curious to see a
>>> network trace if possible.
>>
>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>> attribute is missing. The attribute shouldn't be empty though; when
>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>> permissions. The "system.nfs4_acl" attribute exposes the information
>> on the wire; there is no resonable way to translate that into an ACL
>> on another filesystem, really.
>>
>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>
> getfattr -m - -d nfs/folder -e text gives
>
> # file: nfs/folder/
> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A<E7>\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000
> EVERYONE@\000\000"
>
> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
> empty after all and checking *value == 0 does not tell if there are actually
> ACLs present or not, sorry for the confusion.
>
> You are right, when I mount the exported fs with noacl the problem goes away.
> You already helped me there, thanks.
>
> Still, I think there should be a way to copy up files that actually have no
> ACLs since acl is often the default for ext4 mounts and giving an "Operation
> not supported" for random open(2)s is not a very good way to convey what's
> going on.
It's not hard to come up with a heuristic that determines if a
system.nfs4_acl value is equivalent to a file mode, and to ignore the
attribute in that case. (The file mode is transmitted in its own
attribute already, so actually converting .) That way, overlayfs could
still fail copying up files that have an actual ACL. It's still an
ugly hack ...
Andreas
2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <[email protected]>:
> 2016-12-05 23:58 GMT+01:00 Patrick Plagwitz <[email protected]>:
>> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <[email protected]>:
>>>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>>>>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <[email protected]> wrote:
>>>>>>> Can NFS people comment on this? Where does the nfs4_acl come from?
>>>>>>
>>>>>> This is the interface the NFS client provides for applications to modify
>>>>>> NFSv4 ACLs on servers that support them.
>>>>>
>>>>> Fine, but why are we seeing this xattr on exports where no xattrs are
>>>>> set on the exported fs?
>>>>
>>>> I don't know. I took another look at the original patch and don't see
>>>> any details on the server setup: which server is it (knfsd, ganesha,
>>>> netapp, ...)? How is it configured?
>>>>
>>>>>>> What can overlayfs do if it's a non-empty ACL?
>>>>>>
>>>>>> As little as possible. You can't copy it up, can you? So any attempt
>>>>>> to support it is going to be incomplete.
>>>>>
>>>>> Right.
>>>>>
>>>>>>
>>>>>>> Does knfsd translate posix ACL into NFS acl? If so, we can translate
>>>>>>> back. Should we do a generic POSIX<->NFS acl translator?
>>>>>>
>>>>>> knsd does translate between POSIX and NFSv4 ACLs. It's a complicated
>>>>>
>>>>> This does explain the nfs4_acl xattr on the client. Question: if it's
>>>>> empty, why have it at all?
>>>>
>>>> I'm honestly not sure what's going on there. I'd be curious to see a
>>>> network trace if possible.
>>>
>>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>>> attribute is missing. The attribute shouldn't be empty though; when
>>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>>> permissions. The "system.nfs4_acl" attribute exposes the information
>>> on the wire; there is no resonable way to translate that into an ACL
>>> on another filesystem, really.
>>>
>>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>>
>> getfattr -m - -d nfs/folder -e text gives
>>
>> # file: nfs/folder/
>> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A<E7>\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000<A1>\000\000\000
>> EVERYONE@\000\000"
>>
>> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
>> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
>> empty after all and checking *value == 0 does not tell if there are actually
>> ACLs present or not, sorry for the confusion.
>>
>> You are right, when I mount the exported fs with noacl the problem goes away.
>> You already helped me there, thanks.
>>
>> Still, I think there should be a way to copy up files that actually have no
>> ACLs since acl is often the default for ext4 mounts and giving an "Operation
>> not supported" for random open(2)s is not a very good way to convey what's
>> going on.
>
> It's not hard to come up with a heuristic that determines if a
> system.nfs4_acl value is equivalent to a file mode, and to ignore the
> attribute in that case. (The file mode is transmitted in its own
> attribute already, so actually converting .) That way, overlayfs could
> still fail copying up files that have an actual ACL. It's still an
> ugly hack ...
Actually, that kind of heuristic would make sense in the NFS client
which could then hide the "system.nfs4_acl" attribute.
Andreas
On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
<[email protected]> wrote:
> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <[email protected]>:
>> It's not hard to come up with a heuristic that determines if a
>> system.nfs4_acl value is equivalent to a file mode, and to ignore the
>> attribute in that case. (The file mode is transmitted in its own
>> attribute already, so actually converting .) That way, overlayfs could
>> still fail copying up files that have an actual ACL. It's still an
>> ugly hack ...
>
> Actually, that kind of heuristic would make sense in the NFS client
> which could then hide the "system.nfs4_acl" attribute.
Even simpler would be if knfsd didn't send the attribute if not
necessary. Looks like there's code actively creating the nfs4_acl on
the wire even if the filesystem had none:
pacl = get_acl(inode, ACL_TYPE_ACCESS);
if (!pacl)
pacl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
What's the point?
Thanks,
Miklos
On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi <[email protected]> wrote:
> On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> <[email protected]> wrote:
>> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <[email protected]>:
>
>>> It's not hard to come up with a heuristic that determines if a
>>> system.nfs4_acl value is equivalent to a file mode, and to ignore the
>>> attribute in that case. (The file mode is transmitted in its own
>>> attribute already, so actually converting .) That way, overlayfs could
>>> still fail copying up files that have an actual ACL. It's still an
>>> ugly hack ...
>>
>> Actually, that kind of heuristic would make sense in the NFS client
>> which could then hide the "system.nfs4_acl" attribute.
>
> Even simpler would be if knfsd didn't send the attribute if not
> necessary. Looks like there's code actively creating the nfs4_acl on
> the wire even if the filesystem had none:
>
> pacl = get_acl(inode, ACL_TYPE_ACCESS);
> if (!pacl)
> pacl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
>
> What's the point?
That's how the protocol is specified. (I'm not saying that that's very helpful.)
Andreas
On Tue, Dec 06, 2016 at 02:18:31PM +0100, Andreas Gruenbacher wrote:
> On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi <[email protected]> wrote:
> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > <[email protected]> wrote:
> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <[email protected]>:
> >
> >>> It's not hard to come up with a heuristic that determines if a
> >>> system.nfs4_acl value is equivalent to a file mode, and to ignore the
> >>> attribute in that case. (The file mode is transmitted in its own
> >>> attribute already, so actually converting .) That way, overlayfs could
> >>> still fail copying up files that have an actual ACL. It's still an
> >>> ugly hack ...
> >>
> >> Actually, that kind of heuristic would make sense in the NFS client
> >> which could then hide the "system.nfs4_acl" attribute.
> >
> > Even simpler would be if knfsd didn't send the attribute if not
> > necessary. Looks like there's code actively creating the nfs4_acl on
> > the wire even if the filesystem had none:
> >
> > pacl = get_acl(inode, ACL_TYPE_ACCESS);
> > if (!pacl)
> > pacl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
> >
> > What's the point?
>
> That's how the protocol is specified.
Yep, even if we could make that change to nfsd it wouldn't help the
client with the large number of other servers that are out there
(including older knfsd's).
--b.
> (I'm not saying that that's very helpful.)
>
> Andreas