linux-kernel - Re: [PATCH 0/7] overlay filesystem: request for inclusion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k4bjd8pc.fsf@tucsk.pomaz.szeredi.hu>
Date:	Fri, 15 Jul 2011 14:33:51 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	"J. R. Okajima" <hooanon05@...oo.co.jp>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	NeilBrown <neilb@...e.de>, viro@...IV.linux.org.uk,
	torvalds@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, apw@...onical.com, nbd@...nwrt.org,
	hramrach@...trum.cz, jordipujolp@...il.com, ezk@....cs.sunysb.edu
Subject: Re: [PATCH 0/7] overlay filesystem: request for inclusion

"J. R. Okajima" <hooanon05@...oo.co.jp> writes:

> Miklos Szeredi:
>> Here's a patch to document these limitations.
>
> If you want covering limitations as possible, I'd suggest you to add
> these.
> - Some versions of GNU find(1) may produce a warning about the link
>   count of a directory. When a sub dir exists, find(1) expects it
>   increments the link count of the parent dir.

st_nlink==1 for directories is widely accepted way of saying that the
number of subdirectories is unknown.  Various filesystems already do
this, and versions of GNU utils that I have come across accept it.

> - If I remember correctly, Valerie Aurora has pointed out that
>   open(O_RDONLY) + fchmod() will work correctly in UnionMount.
>   It is true in overlayfs too?

Neither union-mounts nor overlayfs can handle this case.

I hadn't thought about this case, so overlayfs would modify the lower
filesystem in that case, which is a no-no.

Following patch fixes this and return -EROFS for the above case.  I also
updated the non-standard section in the docs.

Thanks,
Miklos
----

Subject: ovl: make lower mount read-only

From: Miklos Szeredi <mszeredi@...e.cz>

If a file only existing on the lower fs is operned for O_RDONLY and
fchmod/fchown/etc is performed on the open file then this will modify
the lower fs, which is not what we want.

Copying up at this point is not possible.  The best solution is to
return an error for this corner case and hope applications are not
relying on it.

Reported-by: "J. R. Okajima" <hooanon05@...oo.co.jp>
Signed-off-by: Miklos Szeredi <mszeredi@...e.cz>
---
 fs/overlayfs/super.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-2.6/fs/overlayfs/super.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/super.c	2011-07-15 12:48:03.000000000 +0200
+++ linux-2.6/fs/overlayfs/super.c	2011-07-15 13:47:35.000000000 +0200
@@ -569,6 +569,12 @@ static int ovl_fill_super(struct super_b
 		goto out_put_upper_mnt;
 	}
 
+	/*
+	 * Make lower_mnt R/O.  That way fchmod/fchown on lower file
+	 * will fail instead of modifying lower fs.
+	 */
+	ufs->lower_mnt->mnt_flags |= MNT_READONLY;
+
 	/* If the upper fs is r/o, we mark overlayfs r/o too */
 	if (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY)
 		sb->s_flags |= MS_RDONLY;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/