[PATCH] ovl: copy up files with incompatible xattrs

[PATCH] ovl: copy up files with incompatible xattrs

[Goldwyn Rodrigues]

From: Fabian Vogt <fvogt@suse.com>

xattrs are not guarantees to be compatible across different filesystems.
Operations which lead to copying of files to the upper layer fail with an
"Operation not supported" error from the filesystem if a xattr could not be
written in the upper layer. We can safely ignore "system" xattrs.

One easy to hit example is using NFS as a read-only lower layer and !NFS as
upper layer to store changes. Files on NFS can have the "system.nfs4_acl"
xattr set to make the full information available to userspace, but it cannot
be written to on other filesystems.

Signed-off-by: Fabian Vogt <fvogt@suse.com>
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/overlayfs/copy_up.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index d855f508fa20..e7662e9664be 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -125,6 +125,10 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
 			continue; /* Discard */
 		}
 		error = vfs_setxattr(new, name, value, size, 0);
+		if (error == -EOPNOTSUPP && strncmp("system.", name, 7) == 0) {
+			error = 0;
+			continue; /* Discard */
+		}
 		if (error)
 			break;
 	}
-- 
2.16.2

Re: [PATCH] ovl: copy up files with incompatible xattrs

[Goldwyn Rodrigues]

Hi,

Any comments on this?

On 04/16/2018 09:35 AM, Goldwyn Rodrigues wrote:
> From: Fabian Vogt <fvogt@suse.com>
> 
> xattrs are not guarantees to be compatible across different filesystems.
> Operations which lead to copying of files to the upper layer fail with an
> "Operation not supported" error from the filesystem if a xattr could not be
> written in the upper layer. We can safely ignore "system" xattrs.
> 
> One easy to hit example is using NFS as a read-only lower layer and !NFS as
> upper layer to store changes. Files on NFS can have the "system.nfs4_acl"
> xattr set to make the full information available to userspace, but it cannot
> be written to on other filesystems.
> 
> Signed-off-by: Fabian Vogt <fvogt@suse.com>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/overlayfs/copy_up.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index d855f508fa20..e7662e9664be 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -125,6 +125,10 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
>  			continue; /* Discard */
>  		}
>  		error = vfs_setxattr(new, name, value, size, 0);
> +		if (error == -EOPNOTSUPP && strncmp("system.", name, 7) == 0) {
> +			error = 0;
> +			continue; /* Discard */
> +		}
>  		if (error)
>  			break;
>  	}
> 

-- 
Goldwyn

Re: [PATCH] ovl: copy up files with incompatible xattrs

[Vivek Goyal]

On Mon, Apr 16, 2018 at 09:35:25AM -0500, Goldwyn Rodrigues wrote:
> From: Fabian Vogt <fvogt@suse.com>
> 
> xattrs are not guarantees to be compatible across different filesystems.
> Operations which lead to copying of files to the upper layer fail with an
> "Operation not supported" error from the filesystem if a xattr could not be
> written in the upper layer. We can safely ignore "system" xattrs.
> 
> One easy to hit example is using NFS as a read-only lower layer and !NFS as
> upper layer to store changes. Files on NFS can have the "system.nfs4_acl"

I don't know much about nfs4_acl. But name suggests that it stored ACLs
there. So if we ignore these over copy up, does that mean we are not
enforcing ACL policy over copy up. So say some user which was not able
to read a file when it was on lower, might be able to read it after
copy up?

Or I have completely misunderstood it?

Vivek

> xattr set to make the full information available to userspace, but it cannot
> be written to on other filesystems.
> 
> Signed-off-by: Fabian Vogt <fvogt@suse.com>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/overlayfs/copy_up.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index d855f508fa20..e7662e9664be 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -125,6 +125,10 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
>  			continue; /* Discard */
>  		}
>  		error = vfs_setxattr(new, name, value, size, 0);
> +		if (error == -EOPNOTSUPP && strncmp("system.", name, 7) == 0) {
> +			error = 0;
> +			continue; /* Discard */
> +		}
>  		if (error)
>  			break;
>  	}
> -- 
> 2.16.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ovl: copy up files with incompatible xattrs

[Goldwyn Rodrigues]

On 05/17/2018 02:26 PM, Vivek Goyal wrote:
> On Mon, Apr 16, 2018 at 09:35:25AM -0500, Goldwyn Rodrigues wrote:
>> From: Fabian Vogt <fvogt@suse.com>
>>
>> xattrs are not guarantees to be compatible across different filesystems.
>> Operations which lead to copying of files to the upper layer fail with an
>> "Operation not supported" error from the filesystem if a xattr could not be
>> written in the upper layer. We can safely ignore "system" xattrs.
>>
>> One easy to hit example is using NFS as a read-only lower layer and !NFS as
>> upper layer to store changes. Files on NFS can have the "system.nfs4_acl"
> 
> I don't know much about nfs4_acl. But name suggests that it stored ACLs
> there. So if we ignore these over copy up, does that mean we are not
> enforcing ACL policy over copy up. So say some user which was not able
> to read a file when it was on lower, might be able to read it after
> copy up?
> 
> Or I have completely misunderstood it?
> 
> 

As far as I know, all "system" attributes are filesystem specific (even
if they are the same fstype) and cannot be comprehended by other
filesystems. Hence, they can be ignored.

Unfortunately, system.nfs4_acl is a part of protocol and is null most of
the times.

Here is an earlier discussion I found which did not conclude:
https://www.spinics.net/lists/linux-nfs/msg61045.html

-- 
Goldwyn

Re: [PATCH] ovl: copy up files with incompatible xattrs

[Miklos Szeredi]

On Fri, May 18, 2018 at 12:30 AM, Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>
>
> On 05/17/2018 02:26 PM, Vivek Goyal wrote:
>> On Mon, Apr 16, 2018 at 09:35:25AM -0500, Goldwyn Rodrigues wrote:
>>> From: Fabian Vogt <fvogt@suse.com>
>>>
>>> xattrs are not guarantees to be compatible across different filesystems.
>>> Operations which lead to copying of files to the upper layer fail with an
>>> "Operation not supported" error from the filesystem if a xattr could not be
>>> written in the upper layer. We can safely ignore "system" xattrs.
>>>
>>> One easy to hit example is using NFS as a read-only lower layer and !NFS as
>>> upper layer to store changes. Files on NFS can have the "system.nfs4_acl"
>>
>> I don't know much about nfs4_acl. But name suggests that it stored ACLs
>> there. So if we ignore these over copy up, does that mean we are not
>> enforcing ACL policy over copy up. So say some user which was not able
>> to read a file when it was on lower, might be able to read it after
>> copy up?
>>
>> Or I have completely misunderstood it?
>>
>>
>
> As far as I know, all "system" attributes are filesystem specific (even
> if they are the same fstype) and cannot be comprehended by other
> filesystems. Hence, they can be ignored.
>
> Unfortunately, system.nfs4_acl is a part of protocol and is null most of
> the times.
>
> Here is an earlier discussion I found which did not conclude:
> https://www.spinics.net/lists/linux-nfs/msg61045.html

It did have a conclusion, except nobody done anything in that direction:

In certain cases nfs4_acl represents the same permissions as file
mode.  This case can be detected and the nfs4_acl xattr ignored.

As a first step that's definitely something that could help in most
cases.  I'd be reluctant to just ignore copy up errors on system
xattrs generally.

Thanks,
Miklos

Re: [PATCH] ovl: copy up files with incompatible xattrs

[Goldwyn Rodrigues]

On 05/18/2018 02:29 AM, Miklos Szeredi wrote:
> On Fri, May 18, 2018 at 12:30 AM, Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>>
>>
>> On 05/17/2018 02:26 PM, Vivek Goyal wrote:
>>> On Mon, Apr 16, 2018 at 09:35:25AM -0500, Goldwyn Rodrigues wrote:
>>>> From: Fabian Vogt <fvogt@suse.com>
>>>>
>>>> xattrs are not guarantees to be compatible across different filesystems.
>>>> Operations which lead to copying of files to the upper layer fail with an
>>>> "Operation not supported" error from the filesystem if a xattr could not be
>>>> written in the upper layer. We can safely ignore "system" xattrs.
>>>>
>>>> One easy to hit example is using NFS as a read-only lower layer and !NFS as
>>>> upper layer to store changes. Files on NFS can have the "system.nfs4_acl"
>>>
>>> I don't know much about nfs4_acl. But name suggests that it stored ACLs
>>> there. So if we ignore these over copy up, does that mean we are not
>>> enforcing ACL policy over copy up. So say some user which was not able
>>> to read a file when it was on lower, might be able to read it after
>>> copy up?
>>>
>>> Or I have completely misunderstood it?
>>>
>>>
>>
>> As far as I know, all "system" attributes are filesystem specific (even
>> if they are the same fstype) and cannot be comprehended by other
>> filesystems. Hence, they can be ignored.
>>
>> Unfortunately, system.nfs4_acl is a part of protocol and is null most of
>> the times.
>>
>> Here is an earlier discussion I found which did not conclude:
>> https://www.spinics.net/lists/linux-nfs/msg61045.html
> 
> It did have a conclusion, except nobody done anything in that direction:
> 
> In certain cases nfs4_acl represents the same permissions as file
> mode.  This case can be detected and the nfs4_acl xattr ignored.
> 
> As a first step that's definitely something that could help in most
> cases.  I'd be reluctant to just ignore copy up errors on system
> xattrs generally.

It seems nfs4_acl is only interpreted by the server and not the client.
nfs4_acl is carried opaque by the client and decoded only in the
userspace by nfs4-acl-tools. Not sure if there is a simpler way to do it
than to copy code from knfsd to client (or use common codebase, if exists).


-- 
Goldwyn