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]

Date:	Wed, 15 Jun 2011 13:19:12 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Valerie Aurora <val@...consulting.com>,
	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

Alan Cox <alan@...rguk.ukuu.org.uk> writes:

>> 1. Al Viro and Christoph Hellwig bring up the same locking problems
>> *every single time* someone proposes a copy-up in-kernel file system,
>> and *every single time* they are dismissed or hand-waved.  
>
> Perhaps you can detail the locking problem in question ?

Pretty bizarre things can happen when the topology of the underlying
layers change after overlayfs acquired refs to underlying dentries.  I
think this is the case Val is talking about.

Example:

# mount -toverlayfs x -oupperdir=/upper,lowerdir=/lower /ovl
# mkdir -p /upper/a/b
# ls /ovl/a/b
# mv /upper/a/b /upper/
# mv /upper/a /upper/b/
# ls  /ovl/a/b
a

Apparently "a" became its own ancestor.

Overlayfs is careful not to assume anything about child/parent
relationships of underlying dentries.

For example "rmdir /ovl/a/b" will do the following:

  1. find the underlying dentry for "a" -> upper-a
  2. lock upper-a
  3. find the underlying dentry for "b" -> upper-b
  4. verify that upper-b is a child of upper-a
  5. remove upper-b

With the above example it will fail on step 4.  Changes to the
underlying filesystems are not supported and result in undefined
behavior.  But it should never result in BUGs or deadlocks.

The overall locking order is:

-> overlayfs locks
  -> upper fs locks
  -> lower fs locks

Within each filesystem the usual locking rules apply.

One more difficulty is copy-up.  This happens without being protected by
i_mutex on overlayfs.  The rules here are: 

 A. directory renames only succeed if both source and destination are
    only on the upper fs (never copied up)
 B. non-directory renames start with copy-up of source if necessary
 C. copy-up takes i_mutex on upper parent

During copy-up no ancestor will be renamed because of A.

The file being copied up may be moved concurrently, however.  If this
happens then copy-up will acquire i_mutex for either the old or the new
upper parent.  In the latter case the file has already been copied up.
In the former case the file may or may not have been copied up.  The
state of the file is checked after having locked the upper parent and
the copy-up skipped if it has already succeeded.

There may be flaws in the above reasoning or the implementation and
reviews are very welcome.

Thanks,
Miklos
--
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/