Linux-EROFS Archive on lore.kernel.org
 help / color / Atom feed
* Compatibility with overlayfs
@ 2019-11-29 20:22 David Michael
  2019-11-30  1:29 ` Gao Xiang via Linux-erofs
  0 siblings, 1 reply; 5+ messages in thread
From: David Michael @ 2019-11-29 20:22 UTC (permalink / raw)
  To: linux-erofs

Hi,

I tried to test EROFS on Linux 5.4 as the root file system and mounted
a writable overlay (with upper layer on tmpfs) over /etc, but I get
ENODATA errors when attempting to modify files.  For example, adding a
user results in "Failed to take /etc/passwd lock: No data available".
Files can be modified after unlinking and restoring them so they're
created on the upper layer.  This is not necessary with other
read-only file systems (at least squashfs or ext4 with the read-only
feature).  I tried while forcing compact and extended inodes.

Is EROFS intended to be usable as a lower layer with overlayfs?

Thanks.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Compatibility with overlayfs
  2019-11-29 20:22 Compatibility with overlayfs David Michael
@ 2019-11-30  1:29 ` Gao Xiang via Linux-erofs
  2019-11-30  2:13   ` Gao Xiang via Linux-erofs
  0 siblings, 1 reply; 5+ messages in thread
From: Gao Xiang via Linux-erofs @ 2019-11-30  1:29 UTC (permalink / raw)
  To: David Michael; +Cc: linux-erofs

Hi David,

On Fri, Nov 29, 2019 at 03:22:15PM -0500, David Michael wrote:
> Hi,
> 
> I tried to test EROFS on Linux 5.4 as the root file system and mounted
> a writable overlay (with upper layer on tmpfs) over /etc, but I get
> ENODATA errors when attempting to modify files.  For example, adding a
> user results in "Failed to take /etc/passwd lock: No data available".
> Files can be modified after unlinking and restoring them so they're
> created on the upper layer.  This is not necessary with other
> read-only file systems (at least squashfs or ext4 with the read-only
> feature).  I tried while forcing compact and extended inodes.
> 
> Is EROFS intended to be usable as a lower layer with overlayfs?

Yes, and overlayfs were used on our smartphones for development use
only as well. I think it's weird, I will try it on the latest kernel
now, and see if I can reproduce this issue soon...

Thanks for your report!

Thanks,
Gao Xiang

> 
> Thanks.
> 
> David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Compatibility with overlayfs
  2019-11-30  1:29 ` Gao Xiang via Linux-erofs
@ 2019-11-30  2:13   ` Gao Xiang via Linux-erofs
  2019-11-30 15:15     ` David Michael
  0 siblings, 1 reply; 5+ messages in thread
From: Gao Xiang via Linux-erofs @ 2019-11-30  2:13 UTC (permalink / raw)
  To: David Michael; +Cc: linux-erofs

On Sat, Nov 30, 2019 at 09:29:04AM +0800, Gao Xiang via Linux-erofs wrote:
> Hi David,
> 
> On Fri, Nov 29, 2019 at 03:22:15PM -0500, David Michael wrote:
> > Hi,
> > 
> > I tried to test EROFS on Linux 5.4 as the root file system and mounted
> > a writable overlay (with upper layer on tmpfs) over /etc, but I get
> > ENODATA errors when attempting to modify files.  For example, adding a
> > user results in "Failed to take /etc/passwd lock: No data available".
> > Files can be modified after unlinking and restoring them so they're
> > created on the upper layer.  This is not necessary with other
> > read-only file systems (at least squashfs or ext4 with the read-only
> > feature).  I tried while forcing compact and extended inodes.
> > 
> > Is EROFS intended to be usable as a lower layer with overlayfs?
> 
> Yes, and overlayfs were used on our smartphones for development use
> only as well. I think it's weird, I will try it on the latest kernel
> now, and see if I can reproduce this issue soon...
> 
> Thanks for your report!
> 
> Thanks,
> Gao Xiang

I have reproduced this issue -- That is due to erofs will return an
unexpected -ENODATA when calling listxattr without xattr and cause
copy_up fail:

int ovl_copy_xattr(struct dentry *old, struct dentry *new)
{
	ssize_t list_size, size, value_size = 0;
	char *buf, *name, *value = NULL;
	int uninitialized_var(error);
	size_t slen;

	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
	    !(new->d_inode->i_opflags & IOP_XATTR))
		return 0;

	list_size = vfs_listxattr(old, NULL, 0);  <- no xattr
	if (list_size <= 0) {
		if (list_size == -EOPNOTSUPP)
			return 0;
		return list_size;    <- will return -ENODATA
	}

since our products using xattr enabled EROFS with overlayfs,
so we didn't observe this issue before. So could you try
the following patch (If it can resolve the issue, I will
send it for 5.5-rc2 and backport to all stable version)?
Look forward to your feekback.

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a13a78725c57..dd328c87dda7 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -649,8 +649,11 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 	struct listxattr_iter it;
 
 	ret = init_inode_xattrs(d_inode(dentry));
-	if (ret)
+	if (ret) {
+		if (ret == -ENODATA)
+			return 0;
 		return ret;
+	}
 
 	it.dentry = dentry;
 	it.buffer = buffer;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Compatibility with overlayfs
  2019-11-30  2:13   ` Gao Xiang via Linux-erofs
@ 2019-11-30 15:15     ` David Michael
  2019-11-30 16:27       ` Gao Xiang via Linux-erofs
  0 siblings, 1 reply; 5+ messages in thread
From: David Michael @ 2019-11-30 15:15 UTC (permalink / raw)
  To: Gao Xiang; +Cc: linux-erofs

On Fri, Nov 29, 2019 at 9:13 PM Gao Xiang <hsiangkao@aol.com> wrote:
> On Sat, Nov 30, 2019 at 09:29:04AM +0800, Gao Xiang via Linux-erofs wrote:
> > Hi David,
> >
> > On Fri, Nov 29, 2019 at 03:22:15PM -0500, David Michael wrote:
> > > Hi,
> > >
> > > I tried to test EROFS on Linux 5.4 as the root file system and mounted
> > > a writable overlay (with upper layer on tmpfs) over /etc, but I get
> > > ENODATA errors when attempting to modify files.  For example, adding a
> > > user results in "Failed to take /etc/passwd lock: No data available".
> > > Files can be modified after unlinking and restoring them so they're
> > > created on the upper layer.  This is not necessary with other
> > > read-only file systems (at least squashfs or ext4 with the read-only
> > > feature).  I tried while forcing compact and extended inodes.
> > >
> > > Is EROFS intended to be usable as a lower layer with overlayfs?
> >
> > Yes, and overlayfs were used on our smartphones for development use
> > only as well. I think it's weird, I will try it on the latest kernel
> > now, and see if I can reproduce this issue soon...
> >
> > Thanks for your report!
> >
> > Thanks,
> > Gao Xiang
>
> I have reproduced this issue -- That is due to erofs will return an
> unexpected -ENODATA when calling listxattr without xattr and cause
> copy_up fail:

Oh, sorry I forgot to mention I had xattrs disabled during my testing.
I confirmed it works with xattrs, so I can use that as a workaround in
binary distros until a fix is upstream.

> since our products using xattr enabled EROFS with overlayfs,
> so we didn't observe this issue before. So could you try
> the following patch (If it can resolve the issue, I will
> send it for 5.5-rc2 and backport to all stable version)?
> Look forward to your feekback.

Yes, I applied the patch and everything works now.

Thanks.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Compatibility with overlayfs
  2019-11-30 15:15     ` David Michael
@ 2019-11-30 16:27       ` Gao Xiang via Linux-erofs
  0 siblings, 0 replies; 5+ messages in thread
From: Gao Xiang via Linux-erofs @ 2019-11-30 16:27 UTC (permalink / raw)
  To: David Michael; +Cc: linux-erofs

On Sat, Nov 30, 2019 at 10:15:25AM -0500, David Michael wrote:
> On Fri, Nov 29, 2019 at 9:13 PM Gao Xiang <hsiangkao@aol.com> wrote:
> > On Sat, Nov 30, 2019 at 09:29:04AM +0800, Gao Xiang via Linux-erofs wrote:
> > > Hi David,
> > >
> > > On Fri, Nov 29, 2019 at 03:22:15PM -0500, David Michael wrote:
> > > > Hi,
> > > >
> > > > I tried to test EROFS on Linux 5.4 as the root file system and mounted
> > > > a writable overlay (with upper layer on tmpfs) over /etc, but I get
> > > > ENODATA errors when attempting to modify files.  For example, adding a
> > > > user results in "Failed to take /etc/passwd lock: No data available".
> > > > Files can be modified after unlinking and restoring them so they're
> > > > created on the upper layer.  This is not necessary with other
> > > > read-only file systems (at least squashfs or ext4 with the read-only
> > > > feature).  I tried while forcing compact and extended inodes.
> > > >
> > > > Is EROFS intended to be usable as a lower layer with overlayfs?
> > >
> > > Yes, and overlayfs were used on our smartphones for development use
> > > only as well. I think it's weird, I will try it on the latest kernel
> > > now, and see if I can reproduce this issue soon...
> > >
> > > Thanks for your report!
> > >
> > > Thanks,
> > > Gao Xiang
> >
> > I have reproduced this issue -- That is due to erofs will return an
> > unexpected -ENODATA when calling listxattr without xattr and cause
> > copy_up fail:
> 
> Oh, sorry I forgot to mention I had xattrs disabled during my testing.
> I confirmed it works with xattrs, so I can use that as a workaround in
> binary distros until a fix is upstream.

Yes, selinux is enabled on Android, so every file has xattr...

I think it could be patched in advance if allowed (it's a trivial but
meaningful fix for noxattr overlayfs scenarios).

Since I've sent pull request for 5.5-rc1 days before. Considering that
it'd better to stay in linux-next for a while, I think it can be upstream
landed in 5.5-rc2 and backport to 5.4 then. (about 2 weeks later)

> 
> > since our products using xattr enabled EROFS with overlayfs,
> > so we didn't observe this issue before. So could you try
> > the following patch (If it can resolve the issue, I will
> > send it for 5.5-rc2 and backport to all stable version)?
> > Look forward to your feekback.
> 
> Yes, I applied the patch and everything works now.

Thanks for your confirmation. It's of great help to catch this. :-)
I'll resend this patch with proper commit message later.

Thanks,
Gao Xiang

> 
> Thanks.
> 
> David

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 20:22 Compatibility with overlayfs David Michael
2019-11-30  1:29 ` Gao Xiang via Linux-erofs
2019-11-30  2:13   ` Gao Xiang via Linux-erofs
2019-11-30 15:15     ` David Michael
2019-11-30 16:27       ` Gao Xiang via Linux-erofs

Linux-EROFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-erofs/0 linux-erofs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-erofs linux-erofs/ https://lore.kernel.org/linux-erofs \
		linux-erofs@lists.ozlabs.org linux-erofs@ozlabs.org
	public-inbox-index linux-erofs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linux-erofs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git