[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=C3=BCnbacher 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 mod=
ify
>>>>> 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 attemp=
t
>>>>> to support it is going to be incomplete.
>>>>
>>>> Right.
>>>>
>>>>>
>>>>>> Does knfsd translate posix ACL into NFS acl? If so, we can translat=
e
>>>>>> 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=3D"\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<A=
1>\000\000\000
> EVERYONE@\000\000"
>
> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=
=3D80.
> It just looked empty because the first byte is 0... Ok, so nfs4_acl is no=
t
> empty after all and checking *value =3D=3D 0 does not tell if there are a=
ctually
> ACLs present or not, sorry for the confusion.
>
> You are right, when I mount the exported fs with noacl the problem goes a=
way.
> 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 "Operat=
ion
> 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=C3=BCnbacher <andreas.gruenbacher@gmai=
l.com>:
> 2016-12-05 23:58 GMT+01:00 Patrick Plagwitz <[email protected]>:
>> On 12/05/2016 08:37 PM, Andreas Gr=C3=BCnbacher 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 mo=
dify
>>>>>> 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 attem=
pt
>>>>>> to support it is going to be incomplete.
>>>>>
>>>>> Right.
>>>>>
>>>>>>
>>>>>>> Does knfsd translate posix ACL into NFS acl? If so, we can transla=
te
>>>>>>> back. Should we do a generic POSIX<->NFS acl translator?
>>>>>>
>>>>>> knsd does translate between POSIX and NFSv4 ACLs. It's a complicate=
d
>>>>>
>>>>> 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=3D"\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=
=3D80.
>> It just looked empty because the first byte is 0... Ok, so nfs4_acl is n=
ot
>> empty after all and checking *value =3D=3D 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 "Opera=
tion
>> 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=C3=BCnbacher
<[email protected]> wrote:
> 2016-12-06 0:19 GMT+01:00 Andreas Gr=C3=BCnbacher <andreas.gruenbacher@gm=
ail.com>:
>> 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 =3D get_acl(inode, ACL_TYPE_ACCESS);
if (!pacl)
pacl =3D 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
On Tue, Dec 06 2016, J. Bruce Fields wrote:
> 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
Hi everyone.....
I have a customer facing this problem, and so stumbled onto the email
thread.
Unfortunately it didn't resolve anything. Maybe I can help kick things
along???
The core problem here is that NFSv4 and ext4 use different and largely
incompatible ACL implementations. There is no way to accurately
translate from one to the other in general (common specific examples
can be converted).
This means that either:
1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
versa) or
2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
that is OK.
Silently not copying the ACLs is probably not a good idea as it might
result in inappropriate permissions being given away. So if the
sysadmin wants this (and some clearly do), they need a way to
explicitly say "I accept the risk". If only standard Unix permissions
are used, there is no risk, so this seems reasonable.
So I would like to propose a new option for overlayfs
nocopyupacl: when overlayfs is copying a file (or directory etc)
from the lower filesystem to the upper filesystem, it does not
copy extended attributes with the "system." prefix. These are
used for storing ACL information and this is sometimes not
compatible between different filesystem types (e.g. ext4 and
NFSv4). Standard Unix ownership permission flags (rwx) *are*
copied so this option does not risk giving away inappropriate
permissions unless the lowerfs uses unusual ACLs.
Miklos: would you find that acceptable?
Thanks,
NeilBrown
If the upper and lower layers use incompatible ACL formats, it is not
possible to copy the ACL xttr from one to the other, so overlayfs
cannot work with them.
This happens particularly with NFSv4 which uses system.nfs4_acl, and
ext4 which uses system.posix_acl_access.
If all ACLs actually make to Unix permissions, then there is no need
to copy up the ACLs, but overlayfs cannot determine this.
So allow the sysadmin it assert that ACLs are not needed with a mount
option
honoracl=off
This causes the ACLs to not be copied, so filesystems with different
ACL formats can be overlaid together.
Signed-off-by: NeilBrown <[email protected]>
---
Documentation/filesystems/overlayfs.txt | 24 ++++++++++++++++++++++++
fs/overlayfs/copy_up.c | 9 +++++++--
fs/overlayfs/dir.c | 2 +-
fs/overlayfs/overlayfs.h | 2 +-
fs/overlayfs/ovl_entry.h | 1 +
fs/overlayfs/super.c | 15 +++++++++++++++
6 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index eef7d9d259e8..7ad675940c93 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -245,6 +245,30 @@ filesystem - future operations on the file are barely noticed by the
overlay filesystem (though an operation on the name of the file such as
rename or unlink will of course be noticed and handled).
+ACL copy-up
+-----------
+
+When a file that only exists on the lower layer is modified it needs
+to be copied up to the upper layer. This means copying the metadata
+and (usually) the data (though see "Metadata only copy up" below).
+One part of the metadata can be problematic: the ACLs.
+
+Now all filesystems support ACLs, and when they do they don't all use
+the same format. A significant conflict appears between POSIX acls
+used on many local filesystems, and NFSv4 ACLs used with NFSv4. There
+two formats are, in general, not inter-convertible.
+
+If a site only uses regular Unix permissions (Read, Write, eXecute by
+User, Group and Other), then as these permissions are compatible with
+all ACLs, there is no need to copy ACLs. overlayfs cannot determine
+if this is the case itself.
+
+For this reason, overlayfs supports a mount option "honoracl=off"
+which causes ACLs, any "system." extended attribute, on the lower
+layer to be ignored and, particularly, not copied to the upper later.
+This allows NFSv4 to be overlaid with a local filesystem, but should
+only be used if the only access controls used on the filesystem are
+Unix permission bits.
Multiple lower layers
---------------------
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 68b3303e4b46..032aa88f21c1 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -39,7 +39,7 @@ static int ovl_ccup_get(char *buf, const struct kernel_param *param)
module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644);
MODULE_PARM_DESC(ovl_check_copy_up, "Obsolete; does nothing");
-int ovl_copy_xattr(struct dentry *old, struct dentry *new)
+int ovl_copy_xattr(struct dentry *old, struct dentry *new, struct ovl_fs *ofs)
{
ssize_t list_size, size, value_size = 0;
char *buf, *name, *value = NULL;
@@ -77,6 +77,10 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
}
list_size -= slen;
+ if (strncmp(name, XATTR_SYSTEM_PREFIX, XATTR_SYSTEM_PREFIX_LEN) == 0 &&
+ !ofs->config.honoracl)
+ continue;
+
if (ovl_is_private_xattr(name))
continue;
retry:
@@ -461,7 +465,8 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
return err;
}
- err = ovl_copy_xattr(c->lowerpath.dentry, temp);
+ err = ovl_copy_xattr(c->lowerpath.dentry, temp,
+ c->dentry->d_sb->s_fs_info);
if (err)
return err;
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 82c129bfe58d..cc8fb9eeb7df 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -368,7 +368,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
if (IS_ERR(opaquedir))
goto out_unlock;
- err = ovl_copy_xattr(upper, opaquedir);
+ err = ovl_copy_xattr(upper, opaquedir, upper->d_sb->s_fs_info);
if (err)
goto out_cleanup;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 9c6018287d57..4a104a4732af 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -422,7 +422,7 @@ int ovl_copy_up(struct dentry *dentry);
int ovl_copy_up_with_data(struct dentry *dentry);
int ovl_copy_up_flags(struct dentry *dentry, int flags);
int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
-int ovl_copy_xattr(struct dentry *old, struct dentry *new);
+int ovl_copy_xattr(struct dentry *old, struct dentry *new, struct ovl_fs *ofs);
int ovl_set_attr(struct dentry *upper, struct kstat *stat);
struct ovl_fh *ovl_encode_real_fh(struct dentry *real, bool is_upper);
int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index ec237035333a..c541e3fed5b9 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -20,6 +20,7 @@ struct ovl_config {
bool nfs_export;
int xino;
bool metacopy;
+ bool honoracl;
};
struct ovl_sb {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 0116735cc321..ceb8fdb7ce14 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -362,6 +362,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
if (ofs->config.metacopy != ovl_metacopy_def)
seq_printf(m, ",metacopy=%s",
ofs->config.metacopy ? "on" : "off");
+ if (!ofs->config.honoracl)
+ seq_puts(m, ",honoracl=off");
return 0;
}
@@ -401,6 +403,8 @@ enum {
OPT_XINO_AUTO,
OPT_METACOPY_ON,
OPT_METACOPY_OFF,
+ OPT_HONORACL_ON,
+ OPT_HONORACL_OFF,
OPT_ERR,
};
@@ -419,6 +423,8 @@ static const match_table_t ovl_tokens = {
{OPT_XINO_AUTO, "xino=auto"},
{OPT_METACOPY_ON, "metacopy=on"},
{OPT_METACOPY_OFF, "metacopy=off"},
+ {OPT_HONORACL_ON, "honoracl=on"},
+ {OPT_HONORACL_OFF, "honoracl=off"},
{OPT_ERR, NULL}
};
@@ -557,6 +563,14 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
config->metacopy = false;
break;
+ case OPT_HONORACL_ON:
+ config->honoracl = true;
+ break;
+
+ case OPT_HONORACL_OFF:
+ config->honoracl = false;
+ break;
+
default:
pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
return -EINVAL;
@@ -1440,6 +1454,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
ofs->config.nfs_export = ovl_nfs_export_def;
ofs->config.xino = ovl_xino_def();
ofs->config.metacopy = ovl_metacopy_def;
+ ofs->config.honoracl = true;
err = ovl_parse_opt((char *) data, &ofs->config);
if (err)
goto out_err;
--
2.14.0.rc0.dirty
Hi Neil,
On 5/1/19 9:35 PM, NeilBrown wrote:
>
> If the upper and lower layers use incompatible ACL formats, it is not
> possible to copy the ACL xttr from one to the other, so overlayfs
attr (?)
> cannot work with them.
> This happens particularly with NFSv4 which uses system.nfs4_acl, and
> ext4 which uses system.posix_acl_access.
>
> If all ACLs actually make to Unix permissions, then there is no need
map (?)
> to copy up the ACLs, but overlayfs cannot determine this.
>
> So allow the sysadmin it assert that ACLs are not needed with a mount
> option
> honoracl=off
> This causes the ACLs to not be copied, so filesystems with different
> ACL formats can be overlaid together.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> Documentation/filesystems/overlayfs.txt | 24 ++++++++++++++++++++++++
> fs/overlayfs/copy_up.c | 9 +++++++--
> fs/overlayfs/dir.c | 2 +-
> fs/overlayfs/overlayfs.h | 2 +-
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/super.c | 15 +++++++++++++++
> 6 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
> index eef7d9d259e8..7ad675940c93 100644
> --- a/Documentation/filesystems/overlayfs.txt
> +++ b/Documentation/filesystems/overlayfs.txt
> @@ -245,6 +245,30 @@ filesystem - future operations on the file are barely noticed by the
> overlay filesystem (though an operation on the name of the file such as
> rename or unlink will of course be noticed and handled).
>
> +ACL copy-up
> +-----------
> +
> +When a file that only exists on the lower layer is modified it needs
> +to be copied up to the upper layer. This means copying the metadata
> +and (usually) the data (though see "Metadata only copy up" below).
> +One part of the metadata can be problematic: the ACLs.
> +
> +Now all filesystems support ACLs, and when they do they don't all use
Not
> +the same format. A significant conflict appears between POSIX acls
ACLs
> +used on many local filesystems, and NFSv4 ACLs used with NFSv4. There
These (or the)
> +two formats are, in general, not inter-convertible.
> +
> +If a site only uses regular Unix permissions (Read, Write, eXecute by
> +User, Group and Other), then as these permissions are compatible with
> +all ACLs, there is no need to copy ACLs. overlayfs cannot determine
> +if this is the case itself.
> +
> +For this reason, overlayfs supports a mount option "honoracl=off"
> +which causes ACLs, any "system." extended attribute, on the lower
> +layer to be ignored and, particularly, not copied to the upper later.
> +This allows NFSv4 to be overlaid with a local filesystem, but should
> +only be used if the only access controls used on the filesystem are
> +Unix permission bits.
>
> Multiple lower layers
> ---------------------
--
~Randy
On Thu, May 2, 2019 at 12:35 AM NeilBrown <[email protected]> wrote:
>
>
> If the upper and lower layers use incompatible ACL formats, it is not
> possible to copy the ACL xttr from one to the other, so overlayfs
> cannot work with them.
> This happens particularly with NFSv4 which uses system.nfs4_acl, and
> ext4 which uses system.posix_acl_access.
>
> If all ACLs actually make to Unix permissions, then there is no need
> to copy up the ACLs, but overlayfs cannot determine this.
>
> So allow the sysadmin it assert that ACLs are not needed with a mount
> option
> honoracl=off
> This causes the ACLs to not be copied, so filesystems with different
> ACL formats can be overlaid together.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> Documentation/filesystems/overlayfs.txt | 24 ++++++++++++++++++++++++
> fs/overlayfs/copy_up.c | 9 +++++++--
> fs/overlayfs/dir.c | 2 +-
> fs/overlayfs/overlayfs.h | 2 +-
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/super.c | 15 +++++++++++++++
> 6 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
> index eef7d9d259e8..7ad675940c93 100644
> --- a/Documentation/filesystems/overlayfs.txt
> +++ b/Documentation/filesystems/overlayfs.txt
> @@ -245,6 +245,30 @@ filesystem - future operations on the file are barely noticed by the
> overlay filesystem (though an operation on the name of the file such as
> rename or unlink will of course be noticed and handled).
>
> +ACL copy-up
> +-----------
> +
> +When a file that only exists on the lower layer is modified it needs
> +to be copied up to the upper layer. This means copying the metadata
> +and (usually) the data (though see "Metadata only copy up" below).
> +One part of the metadata can be problematic: the ACLs.
> +
> +Now all filesystems support ACLs, and when they do they don't all use
> +the same format. A significant conflict appears between POSIX acls
> +used on many local filesystems, and NFSv4 ACLs used with NFSv4. There
> +two formats are, in general, not inter-convertible.
> +
> +If a site only uses regular Unix permissions (Read, Write, eXecute by
> +User, Group and Other), then as these permissions are compatible with
> +all ACLs, there is no need to copy ACLs. overlayfs cannot determine
> +if this is the case itself.
> +
> +For this reason, overlayfs supports a mount option "honoracl=off"
> +which causes ACLs, any "system." extended attribute, on the lower
> +layer to be ignored and, particularly, not copied to the upper later.
> +This allows NFSv4 to be overlaid with a local filesystem, but should
> +only be used if the only access controls used on the filesystem are
> +Unix permission bits.
>
I don't know. On the one hand "system." is not only ACLs.
On the other hand, "honoracl=off" is not the same as -o noacl,
but it sure sounds the same.
I'd be a lot more comfortable with "ignore_xattrs=system.nfs4_acl"
argument takes a comma separated list of xattr prefixes to ignore.
ovl_is_private_xattr() can be generalized to ovl_is_ignored_xattr(),
going over a blacklist of N>=1 which will also be called from
ovl_can_list(), because there is no point in listing the ACLs that
are ignored. right?
Thanks,
Amir.
NeilBrown:
> If the upper and lower layers use incompatible ACL formats, it is not
> possible to copy the ACL xttr from one to the other, so overlayfs
> cannot work with them.
> This happens particularly with NFSv4 which uses system.nfs4_acl, and
> ext4 which uses system.posix_acl_access.
FYI,
Aufs had met the same problem many years ago, and I introduced some
options called ICEX (Ignore Copyup Error on Xattr).
(from the design doc in aufs)
----------------------------------------
For example, the src branch supports ACL but the dst branch doesn't
because the dst branch may natively un-support it or temporary
un-support it due to "noacl" mount option. Of course, the dst branch fs
may NOT return an error even if the XATTR is not supported. It is
totally up to the branch fs.
Anyway when the aufs internal copy-up gets an error from the dst branch
fs, then aufs tries removing the just copied entry and returns the error
to the userspace. The worst case of this situation will be all copy-up
will fail.
For the copy-up operation, there two basic approaches.
- copy the specified XATTR only (by category above), and return the
error unconditionally if it happens.
- copy all XATTR, and ignore the error on the specified category only.
In order to support XATTR and to implement the correct behaviour, aufs
chooses the latter approach and introduces some new branch attributes,
"icexsec", "icexsys", "icextr", "icexusr", and "icexoth".
They correspond to the XATTR namespaces (see above). Additionally, to be
convenient, "icex" is also provided which means all "icex*" attributes
are set (here the word "icex" stands for "ignore copy-error on XATTR").
The meaning of these attributes is to ignore the error from setting
XATTR on that branch.
Note that aufs tries copying all XATTR unconditionally, and ignores the
error from the dst branch according to the specified attributes.
----------------------------------------
But recent nfs4 got changed its behaviour around ACL, and it didn't pass
my local tests. I had posted about that, but got no reply.
J. R. Okajima
On Thu, May 02 2019, Amir Goldstein wrote:
> On Thu, May 2, 2019 at 12:35 AM NeilBrown <[email protected]> wrote:
>>
>>
>> If the upper and lower layers use incompatible ACL formats, it is not
>> possible to copy the ACL xttr from one to the other, so overlayfs
>> cannot work with them.
>> This happens particularly with NFSv4 which uses system.nfs4_acl, and
>> ext4 which uses system.posix_acl_access.
>>
>> If all ACLs actually make to Unix permissions, then there is no need
>> to copy up the ACLs, but overlayfs cannot determine this.
>>
>> So allow the sysadmin it assert that ACLs are not needed with a mount
>> option
>> honoracl=off
>> This causes the ACLs to not be copied, so filesystems with different
>> ACL formats can be overlaid together.
>>
>> Signed-off-by: NeilBrown <[email protected]>
>> ---
>> Documentation/filesystems/overlayfs.txt | 24 ++++++++++++++++++++++++
>> fs/overlayfs/copy_up.c | 9 +++++++--
>> fs/overlayfs/dir.c | 2 +-
>> fs/overlayfs/overlayfs.h | 2 +-
>> fs/overlayfs/ovl_entry.h | 1 +
>> fs/overlayfs/super.c | 15 +++++++++++++++
>> 6 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
>> index eef7d9d259e8..7ad675940c93 100644
>> --- a/Documentation/filesystems/overlayfs.txt
>> +++ b/Documentation/filesystems/overlayfs.txt
>> @@ -245,6 +245,30 @@ filesystem - future operations on the file are barely noticed by the
>> overlay filesystem (though an operation on the name of the file such as
>> rename or unlink will of course be noticed and handled).
>>
>> +ACL copy-up
>> +-----------
>> +
>> +When a file that only exists on the lower layer is modified it needs
>> +to be copied up to the upper layer. This means copying the metadata
>> +and (usually) the data (though see "Metadata only copy up" below).
>> +One part of the metadata can be problematic: the ACLs.
>> +
>> +Now all filesystems support ACLs, and when they do they don't all use
>> +the same format. A significant conflict appears between POSIX acls
>> +used on many local filesystems, and NFSv4 ACLs used with NFSv4. There
>> +two formats are, in general, not inter-convertible.
>> +
>> +If a site only uses regular Unix permissions (Read, Write, eXecute by
>> +User, Group and Other), then as these permissions are compatible with
>> +all ACLs, there is no need to copy ACLs. overlayfs cannot determine
>> +if this is the case itself.
>> +
>> +For this reason, overlayfs supports a mount option "honoracl=off"
>> +which causes ACLs, any "system." extended attribute, on the lower
>> +layer to be ignored and, particularly, not copied to the upper later.
>> +This allows NFSv4 to be overlaid with a local filesystem, but should
>> +only be used if the only access controls used on the filesystem are
>> +Unix permission bits.
>>
>
> I don't know. On the one hand "system." is not only ACLs.
Isn't it? What else goes in "system." "??
"man xattr" says:
Extended system attributes
Extended system attributes are used by the kernel to store system
objects such as Access Control Lists. Read and write access
permissions to system attributes depend on the policy implemented
for each system attribute implemented by filesystems in the
kernel.
so it *allows* things other than ACLs, but doesn't confirm that there
are any.
In the kernel source, "XATTR_SYSTEM_PREFIX" is only used with POSIX acls
and "system.sockprotoname" - which is socket specific and no likely to
be found on a filesystem.
"system.
also appears in
CIFS_XATTR_CIFS_ACL
SMB3_XATTR_CIFS_ACL
F2FS_SYSTEM_ADVISE_NAME
XATTR_NAME_NFSV4_ACL
SYSTEM_ORANGEFS_KEY
which should all use XATTR_SYSTEM_PREFIX ...
So yes, I guess they aren't (quite) all ACLs. Bother.
> On the other hand, "honoracl=off" is not the same as -o noacl,
> but it sure sounds the same.
>
> I'd be a lot more comfortable with "ignore_xattrs=system.nfs4_acl"
> argument takes a comma separated list of xattr prefixes to ignore.
That requires the sysadmin to know a lot more about the internals of the
relevant filesystems.... Maybe that is a good idea, but it feels rather
clunky.
In each of these cases, except maybe POSIX_ACLs, it doesn't make sense
to copy-up the "system." xattr unless it is the exact same filesystem
type.
So if given a "noacl" flag (or similar), ignoring copy-up failure for
all "system." attributes is probably the right thing to do, as ACLs are
the only system. attribute for which it can make any sense at all to
copy them.
Thanks,
NeilBrown
>
> ovl_is_private_xattr() can be generalized to ovl_is_ignored_xattr(),
> going over a blacklist of N>=1 which will also be called from
> ovl_can_list(), because there is no point in listing the ACLs that
> are ignored. right?
>
> Thanks,
> Amir.
On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> On Tue, Dec 06 2016, J. Bruce Fields wrote:
>
> > 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
>
> Hi everyone.....
> I have a customer facing this problem, and so stumbled onto the email
> thread.
> Unfortunately it didn't resolve anything. Maybe I can help kick things
> along???
>
> The core problem here is that NFSv4 and ext4 use different and largely
> incompatible ACL implementations. There is no way to accurately
> translate from one to the other in general (common specific examples
> can be converted).
>
> This means that either:
> 1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
> versa) or
> 2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
> that is OK.
>
> Silently not copying the ACLs is probably not a good idea as it might
> result in inappropriate permissions being given away. So if the
> sysadmin wants this (and some clearly do), they need a way to
> explicitly say "I accept the risk".
So, I feel like silently copying ACLs up *also* carries a risk, if that
means switching from server-enforcement to client-enforcement of those
permissions.
Sorry, I know we had another thread recently about permissions in this
situation, and I've forgotten the conclusion.
Out of curiosity, what's done with selinux labels?
--b.
> If only standard Unix permissions
> are used, there is no risk, so this seems reasonable.
>
> So I would like to propose a new option for overlayfs
> nocopyupacl: when overlayfs is copying a file (or directory etc)
> from the lower filesystem to the upper filesystem, it does not
> copy extended attributes with the "system." prefix. These are
> used for storing ACL information and this is sometimes not
> compatible between different filesystem types (e.g. ext4 and
> NFSv4). Standard Unix ownership permission flags (rwx) *are*
> copied so this option does not risk giving away inappropriate
> permissions unless the lowerfs uses unusual ACLs.
>
>
> Miklos: would you find that acceptable?
>
> Thanks,
> NeilBrown
>
>
On Fri, May 3, 2019 at 12:03 PM J. Bruce Fields <[email protected]> wrote:
>
> On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> > On Tue, Dec 06 2016, J. Bruce Fields wrote:
> >
> > > 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
> >
> > Hi everyone.....
> > I have a customer facing this problem, and so stumbled onto the email
> > thread.
> > Unfortunately it didn't resolve anything. Maybe I can help kick things
> > along???
> >
> > The core problem here is that NFSv4 and ext4 use different and largely
> > incompatible ACL implementations. There is no way to accurately
> > translate from one to the other in general (common specific examples
> > can be converted).
> >
> > This means that either:
> > 1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
> > versa) or
> > 2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
> > that is OK.
> >
> > Silently not copying the ACLs is probably not a good idea as it might
> > result in inappropriate permissions being given away. So if the
> > sysadmin wants this (and some clearly do), they need a way to
> > explicitly say "I accept the risk".
>
> So, I feel like silently copying ACLs up *also* carries a risk, if that
> means switching from server-enforcement to client-enforcement of those
> permissions.
>
> Sorry, I know we had another thread recently about permissions in this
> situation, and I've forgotten the conclusion.
>
> Out of curiosity, what's done with selinux labels?
>
overlayfs calls security_inode_copy_up_xattr(name) which
can fail (<0) allow (0) or skip(1).
selinux_inode_copy_up_xattr() as well as smack_inode_copy_up_xattr()
skip their own xattr on copy up and fail any other xattr copy up.
Thanks,
Amir.
On Fri, May 03, 2019 at 01:26:01PM -0400, Amir Goldstein wrote:
> On Fri, May 3, 2019 at 12:03 PM J. Bruce Fields <[email protected]> wrote:
> >
> > On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> > > On Tue, Dec 06 2016, J. Bruce Fields wrote:
> > >
> > > > 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
> > >
> > > Hi everyone.....
> > > I have a customer facing this problem, and so stumbled onto the email
> > > thread.
> > > Unfortunately it didn't resolve anything. Maybe I can help kick things
> > > along???
> > >
> > > The core problem here is that NFSv4 and ext4 use different and largely
> > > incompatible ACL implementations. There is no way to accurately
> > > translate from one to the other in general (common specific examples
> > > can be converted).
> > >
> > > This means that either:
> > > 1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
> > > versa) or
> > > 2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
> > > that is OK.
> > >
> > > Silently not copying the ACLs is probably not a good idea as it might
> > > result in inappropriate permissions being given away. So if the
> > > sysadmin wants this (and some clearly do), they need a way to
> > > explicitly say "I accept the risk".
> >
> > So, I feel like silently copying ACLs up *also* carries a risk, if that
> > means switching from server-enforcement to client-enforcement of those
> > permissions.
> >
> > Sorry, I know we had another thread recently about permissions in this
> > situation, and I've forgotten the conclusion.
> >
> > Out of curiosity, what's done with selinux labels?
> >
>
> overlayfs calls security_inode_copy_up_xattr(name) which
> can fail (<0) allow (0) or skip(1).
>
> selinux_inode_copy_up_xattr() as well as smack_inode_copy_up_xattr()
> skip their own xattr on copy up and fail any other xattr copy up.
If it's OK to silently skip copying up security labels, maybe it's OK to
silently skip NFSv4 ACLs too?
--b.
On Fri, May 3, 2019 at 1:31 PM J. Bruce Fields <[email protected]> wrote:
>
> On Fri, May 03, 2019 at 01:26:01PM -0400, Amir Goldstein wrote:
> > On Fri, May 3, 2019 at 12:03 PM J. Bruce Fields <[email protected]> wrote:
> > >
> > > On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> > > > On Tue, Dec 06 2016, J. Bruce Fields wrote:
> > > >
> > > > > 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
> > > >
> > > > Hi everyone.....
> > > > I have a customer facing this problem, and so stumbled onto the email
> > > > thread.
> > > > Unfortunately it didn't resolve anything. Maybe I can help kick things
> > > > along???
> > > >
> > > > The core problem here is that NFSv4 and ext4 use different and largely
> > > > incompatible ACL implementations. There is no way to accurately
> > > > translate from one to the other in general (common specific examples
> > > > can be converted).
> > > >
> > > > This means that either:
> > > > 1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
> > > > versa) or
> > > > 2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
> > > > that is OK.
> > > >
> > > > Silently not copying the ACLs is probably not a good idea as it might
> > > > result in inappropriate permissions being given away. So if the
> > > > sysadmin wants this (and some clearly do), they need a way to
> > > > explicitly say "I accept the risk".
> > >
> > > So, I feel like silently copying ACLs up *also* carries a risk, if that
> > > means switching from server-enforcement to client-enforcement of those
> > > permissions.
> > >
> > > Sorry, I know we had another thread recently about permissions in this
> > > situation, and I've forgotten the conclusion.
> > >
> > > Out of curiosity, what's done with selinux labels?
> > >
> >
> > overlayfs calls security_inode_copy_up_xattr(name) which
> > can fail (<0) allow (0) or skip(1).
> >
> > selinux_inode_copy_up_xattr() as well as smack_inode_copy_up_xattr()
> > skip their own xattr on copy up and fail any other xattr copy up.
>
> If it's OK to silently skip copying up security labels, maybe it's OK to
> silently skip NFSv4 ACLs too?
>
I think overlayfs inode security context is taken from overlayfs
mount parameters (i.e. per container context) and therefore
the lower security. xattr are ignored (CC Vivek).
Thanks,
Amir.
On Fri, May 03 2019, J. Bruce Fields wrote:
> On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
>> On Tue, Dec 06 2016, J. Bruce Fields wrote:
>>
>> > 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
>>
>> Hi everyone.....
>> I have a customer facing this problem, and so stumbled onto the email
>> thread.
>> Unfortunately it didn't resolve anything. Maybe I can help kick things
>> along???
>>
>> The core problem here is that NFSv4 and ext4 use different and largely
>> incompatible ACL implementations. There is no way to accurately
>> translate from one to the other in general (common specific examples
>> can be converted).
>>
>> This means that either:
>> 1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
>> versa) or
>> 2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
>> that is OK.
>>
>> Silently not copying the ACLs is probably not a good idea as it might
>> result in inappropriate permissions being given away. So if the
>> sysadmin wants this (and some clearly do), they need a way to
>> explicitly say "I accept the risk".
>
> So, I feel like silently copying ACLs up *also* carries a risk, if that
> means switching from server-enforcement to client-enforcement of those
> permissions.
Interesting perspective .... though doesn't NFSv4 explicitly allow
client-side ACL enforcement in the case of delegations?
Not sure how relevant that is....
It seems to me we have two options:
1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
recommend people use NFSv3, or
2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
2a/ always - and ignore all other acls and probably all system. xattrs,
or
2b/ based on a mount option that might be
2bi/ general "noacl" or might be
2bii/ explicit "noxattr=system.nfs4acl"
I think that continuing to discuss the miniature of the options isn't
going to help. No solution is perfect - we just need to clearly
document the implications of whatever we come up with.
I lean towards 2a, but I be happy with with any '2' and '1' won't kill
me.
Do we have a vote? Or does someone make an executive decision??
NeilBrown
On Fri, May 3, 2019 at 11:35 AM J. Bruce Fields <[email protected]> wrote:
>
> On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> > Silently not copying the ACLs is probably not a good idea as it might
> > result in inappropriate permissions being given away. So if the
> > sysadmin wants this (and some clearly do), they need a way to
> > explicitly say "I accept the risk".
>
> So, I feel like silently copying ACLs up *also* carries a risk, if that
> means switching from server-enforcement to client-enforcement of those
> permissions.
That's not correct: permissions are checked on the overlay layer,
regardless of where the actual file resides. For filesystems using a
server enforced permission model that means possibly different
permissions for accesses through overlayfs than for accesses without
overlayfs. Apparently this is missing from the documentation and
definitely needs to be added.
So I think it's perfectly fine to allow copying up ACLs, as long as
the ACL is representable on the upper fs. If that cannot be ensured,
then the only sane thing to do is to disable ACL checking across the
overlay ("noacl" option).
Thanks,
Miklos
On Tue, May 07, 2019 at 04:07:21AM -0400, Miklos Szeredi wrote:
> On Fri, May 3, 2019 at 11:35 AM J. Bruce Fields <[email protected]> wrote:
> >
> > On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
>
> > > Silently not copying the ACLs is probably not a good idea as it might
> > > result in inappropriate permissions being given away. So if the
> > > sysadmin wants this (and some clearly do), they need a way to
> > > explicitly say "I accept the risk".
> >
> > So, I feel like silently copying ACLs up *also* carries a risk, if that
> > means switching from server-enforcement to client-enforcement of those
> > permissions.
>
> That's not correct: permissions are checked on the overlay layer,
> regardless of where the actual file resides. For filesystems using a
> server enforced permission model that means possibly different
> permissions for accesses through overlayfs than for accesses without
> overlayfs. Apparently this is missing from the documentation and
> definitely needs to be added.
Well, we did have a thread on this pretty recently, I think, and I'm
just not remembering the conclusion. Yes, it'd be nice to have this
documented.
In the case of NFSv4 ACLs, we not only lack storage for them, we don't
even have code to evaluate them.
--b.
> So I think it's perfectly fine to allow copying up ACLs, as long as
> the ACL is representable on the upper fs. If that cannot be ensured,
> then the only sane thing to do is to disable ACL checking across the
> overlay ("noacl" option).
On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> Interesting perspective .... though doesn't NFSv4 explicitly allow
> client-side ACL enforcement in the case of delegations?
Not really. What you're probably thinking of is the single ACE that the
server can return on granting a delegation, that tells the client it can
skip the ACCESS check for users matching that ACE. It's unclear how
useful that is. It's currently unused by the Linux client and server.
> Not sure how relevant that is....
>
> It seems to me we have two options:
> 1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> recommend people use NFSv3, or
> 2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
> 2a/ always - and ignore all other acls and probably all system. xattrs,
> or
> 2b/ based on a mount option that might be
> 2bi/ general "noacl" or might be
> 2bii/ explicit "noxattr=system.nfs4acl"
>
> I think that continuing to discuss the miniature of the options isn't
> going to help. No solution is perfect - we just need to clearly
> document the implications of whatever we come up with.
>
> I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> me.
I guess I'd also lean towards 2a.
I don't think it applies to posix acls, as overlayfs is capable of
copying those up and evaluating them on its own.
--b.
>
> Do we have a vote? Or does someone make an executive decision??
>
> NeilBrown
On Fri, May 10, 2019 at 04:09:41PM -0400, J. Bruce Fields wrote:
> On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> > Interesting perspective .... though doesn't NFSv4 explicitly allow
> > client-side ACL enforcement in the case of delegations?
>
> Not really. What you're probably thinking of is the single ACE that the
> server can return on granting a delegation, that tells the client it can
> skip the ACCESS check for users matching that ACE. It's unclear how
> useful that is. It's currently unused by the Linux client and server.
>
> > Not sure how relevant that is....
> >
> > It seems to me we have two options:
> > 1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> > recommend people use NFSv3, or
> > 2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
> > 2a/ always - and ignore all other acls and probably all system. xattrs,
> > or
> > 2b/ based on a mount option that might be
> > 2bi/ general "noacl" or might be
> > 2bii/ explicit "noxattr=system.nfs4acl"
> >
> > I think that continuing to discuss the miniature of the options isn't
> > going to help. No solution is perfect - we just need to clearly
> > document the implications of whatever we come up with.
> >
> > I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> > me.
>
> I guess I'd also lean towards 2a.
>
> I don't think it applies to posix acls, as overlayfs is capable of
> copying those up and evaluating them on its own.
POSIX acls are evaluated and copied up.
I guess same goes for "security.*" attributes, that are evaluated on MAC checks.
I think it would be safe to ignore failure to copy up anything else. That seems
a bit saner than just blacklisting nfs4_acl...
Something like the following untested patch.
Thanks,
Miklos
---
fs/overlayfs/copy_up.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -36,6 +36,13 @@ static int ovl_ccup_get(char *buf, const
module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644);
MODULE_PARM_DESC(check_copy_up, "Obsolete; does nothing");
+static bool ovl_must_copy_xattr(const char *name)
+{
+ return !strcmp(name, XATTR_POSIX_ACL_ACCESS) ||
+ !strcmp(name, XATTR_POSIX_ACL_DEFAULT) ||
+ !strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN);
+}
+
int ovl_copy_xattr(struct dentry *old, struct dentry *new)
{
ssize_t list_size, size, value_size = 0;
@@ -107,8 +114,13 @@ int ovl_copy_xattr(struct dentry *old, s
continue; /* Discard */
}
error = vfs_setxattr(new, name, value, size, 0);
- if (error)
- break;
+ if (error) {
+ if (ovl_must_copy_xattr(name))
+ break;
+
+ /* Ignore failure to copy unknown xattrs */
+ error = 0;
+ }
}
kfree(value);
out:
On Wed, Sep 18, 2019 at 11:07:31AM +0200, Miklos Szeredi wrote:
> On Fri, May 10, 2019 at 04:09:41PM -0400, J. Bruce Fields wrote:
> > On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> > > Interesting perspective .... though doesn't NFSv4 explicitly allow
> > > client-side ACL enforcement in the case of delegations?
> >
> > Not really. What you're probably thinking of is the single ACE that the
> > server can return on granting a delegation, that tells the client it can
> > skip the ACCESS check for users matching that ACE. It's unclear how
> > useful that is. It's currently unused by the Linux client and server.
> >
> > > Not sure how relevant that is....
> > >
> > > It seems to me we have two options:
> > > 1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> > > recommend people use NFSv3, or
> > > 2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
> > > 2a/ always - and ignore all other acls and probably all system. xattrs,
> > > or
> > > 2b/ based on a mount option that might be
> > > 2bi/ general "noacl" or might be
> > > 2bii/ explicit "noxattr=system.nfs4acl"
> > >
> > > I think that continuing to discuss the miniature of the options isn't
> > > going to help. No solution is perfect - we just need to clearly
> > > document the implications of whatever we come up with.
> > >
> > > I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> > > me.
> >
> > I guess I'd also lean towards 2a.
> >
> > I don't think it applies to posix acls, as overlayfs is capable of
> > copying those up and evaluating them on its own.
>
> POSIX acls are evaluated and copied up.
>
> I guess same goes for "security.*" attributes, that are evaluated on MAC checks.
>
> I think it would be safe to ignore failure to copy up anything else. That seems
> a bit saner than just blacklisting nfs4_acl...
>
> Something like the following untested patch.
It seems at least simple to implement and explain.
> fs/overlayfs/copy_up.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -36,6 +36,13 @@ static int ovl_ccup_get(char *buf, const
> module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644);
> MODULE_PARM_DESC(check_copy_up, "Obsolete; does nothing");
>
> +static bool ovl_must_copy_xattr(const char *name)
> +{
> + return !strcmp(name, XATTR_POSIX_ACL_ACCESS) ||
> + !strcmp(name, XATTR_POSIX_ACL_DEFAULT) ||
> + !strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN);
> +}
> +
> int ovl_copy_xattr(struct dentry *old, struct dentry *new)
> {
> ssize_t list_size, size, value_size = 0;
> @@ -107,8 +114,13 @@ int ovl_copy_xattr(struct dentry *old, s
> continue; /* Discard */
> }
> error = vfs_setxattr(new, name, value, size, 0);
> - if (error)
> - break;
> + if (error) {
Can we check for EOPNOTSUPP instead of any error?
Maybe we're copying up a user xattr to a filesystem that's perfectly
capable of supporting those. And maybe there's a disk error or we run
out of disk space or something. Then I'd rather get EIO or ENOSPC than
silently fail to copy some xattrs.
--b.
> + if (ovl_must_copy_xattr(name))
> + break;
> +
> + /* Ignore failure to copy unknown xattrs */
> + error = 0;
> + }
> }
> kfree(value);
> out: