Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1535929 > unrolled thread

Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Miklos Szeredi <miklos@szeredi.hu> - 2016-12-05 10:30 +0100
    Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir "J. Bruce Fields" <bfields@fieldses.org> - 2016-12-05 16:30 +0100
      Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Miklos Szeredi <miklos@szeredi.hu> - 2016-12-05 16:40 +0100
        Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir "J. Bruce Fields" <bfields@fieldses.org> - 2016-12-05 17:30 +0100
          Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Patrick Plagwitz <Patrick_Plagwitz@web.de> - 2016-12-05 19:30 +0100
          Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Andreas Grünbacher <andreas.gruenbacher@gmail.com> - 2016-12-05 20:40 +0100
            Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Patrick Plagwitz <Patrick_Plagwitz@web.de> - 2016-12-06 00:00 +0100
              Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Andreas Grünbacher <andreas.gruenbacher@gmail.com> - 2016-12-06 00:30 +0100
                Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Miklos Szeredi <miklos@szeredi.hu> - 2016-12-06 11:10 +0100
                  Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Andreas Gruenbacher <agruenba@redhat.com> - 2016-12-06 14:20 +0100
                    Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir "J. Bruce Fields" <bfields@fieldses.org> - 2016-12-06 20:00 +0100
              Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir Andreas Grünbacher <andreas.gruenbacher@gmail.com> - 2016-12-06 00:30 +0100

#1535929 — Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir

[Added a few more CCs]

On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
<Patrick_Plagwitz@web.de> 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 <patrick_plagwitz@web.de>
> ---
> 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

[toc] | [next] | [standalone]

#1536164

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
> <Patrick_Plagwitz@web.de> 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 <patrick_plagwitz@web.de>
> > ---
> > 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.

[toc] | [prev] | [next] | [standalone]

#1536173

On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields <bfields@fieldses.org> 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

[toc] | [prev] | [next] | [standalone]

#1536216

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 <bfields@fieldses.org> 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.

[toc] | [prev] | [next] | [standalone]

#1536313

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 <bfields@fieldses.org> 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)]

[toc] | [prev] | [next] | [standalone]

#1536359

2016-12-05 17:25 GMT+01:00 J. Bruce Fields <bfields@fieldses.org>:
> 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 <bfields@fieldses.org> 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

[toc] | [prev] | [next] | [standalone]

#1536509

On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <bfields@fieldses.org>:
>> 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 <bfields@fieldses.org> 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.

[toc] | [prev] | [next] | [standalone]

#1536522

2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <andreas.gruenbacher@gmail.com>:
> 2016-12-05 23:58 GMT+01:00 Patrick Plagwitz <Patrick_Plagwitz@web.de>:
>> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <bfields@fieldses.org>:
>>>> 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 <bfields@fieldses.org> 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

[toc] | [prev] | [next] | [standalone]

#1536813

On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
<andreas.gruenbacher@gmail.com> wrote:
> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <andreas.gruenbacher@gmail.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 = get_acl(inode, ACL_TYPE_ACCESS);
    if (!pacl)
        pacl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);

What's the point?

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]

#1536971

On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> <andreas.gruenbacher@gmail.com> wrote:
>> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <andreas.gruenbacher@gmail.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 = 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

[toc] | [prev] | [next] | [standalone]

#1537211

On Tue, Dec 06, 2016 at 02:18:31PM +0100, Andreas Gruenbacher wrote:
> On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > <andreas.gruenbacher@gmail.com> wrote:
> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher <andreas.gruenbacher@gmail.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 = 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

[toc] | [prev] | [next] | [standalone]

#1536523

2016-12-05 23:58 GMT+01:00 Patrick Plagwitz <Patrick_Plagwitz@web.de>:
> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields <bfields@fieldses.org>:
>>> 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 <bfields@fieldses.org> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web