3.16.63-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: "Eric W. Biederman" ebiederm@xmission.com
commit 9c8e0a1b683525464a2abe9fb4b54404a50ed2b4 upstream.
Timothy Baldwin timbaldwin@fastmail.co.uk wrote:
As per mount_namespaces(7) unprivileged users should not be able to look under mount points:
Mounts that come as a single unit from more privileged mount are locked together and may not be separated in a less privileged mount namespace.
However they can:
- Create a mount namespace.
- In the mount namespace open a file descriptor to the parent of a mount point.
- Destroy the mount namespace.
- Use the file descriptor to look under the mount point.
I have reproduced this with Linux 4.16.18 and Linux 4.18-rc8.
The setup:
$ sudo sysctl kernel.unprivileged_userns_clone=1 kernel.unprivileged_userns_clone = 1 $ mkdir -p A/B/Secret $ sudo mount -t tmpfs hide A/B
"Secret" is indeed hidden as expected:
$ ls -lR A A: total 0 drwxrwxrwt 2 root root 40 Feb 12 21:08 B
A/B: total 0
The attack revealing "Secret":
$ unshare -Umr sh -c "exec unshare -m ls -lR /proc/self/fd/4/ 4<A" /proc/self/fd/4/: total 0 drwxr-xr-x 3 root root 60 Feb 12 21:08 B
/proc/self/fd/4/B: total 0 drwxr-xr-x 2 root root 40 Feb 12 21:08 Secret
/proc/self/fd/4/B/Secret: total 0
I tracked this down to put_mnt_ns running passing UMOUNT_SYNC and disconnecting all of the mounts in a mount namespace. Fix this by factoring drop_mounts out of drop_collected_mounts and passing 0 instead of UMOUNT_SYNC.
There are two possible behavior differences that result from this. - No longer setting UMOUNT_SYNC will no longer set MNT_SYNC_UMOUNT on the vfsmounts being unmounted. This effects the lazy rcu walk by kicking the walk out of rcu mode and forcing it to be a non-lazy walk. - No longer disconnecting locked mounts will keep some mounts around longer as they stay because the are locked to other mounts.
There are only two users of drop_collected mounts: audit_tree.c and put_mnt_ns.
In audit_tree.c the mounts are private and there are no rcu lazy walks only calls to iterate_mounts. So the changes should have no effect except for a small timing effect as the connected mounts are disconnected.
In put_mnt_ns there may be references from process outside the mount namespace to the mounts. So the mounts remaining connected will be the bug fix that is needed. That rcu walks are allowed to continue appears not to be a problem especially as the rcu walk change was about an implementation detail not about semantics.
Fixes: 5ff9d8a65ce8 ("vfs: Lock in place mounts from more privileged users") Reported-by: Timothy Baldwin timbaldwin@fastmail.co.uk Tested-by: Timothy Baldwin timbaldwin@fastmail.co.uk Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com Signed-off-by: Ben Hutchings ben@decadent.org.uk --- fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
--- a/fs/namespace.c +++ b/fs/namespace.c @@ -1641,7 +1641,7 @@ void drop_collected_mounts(struct vfsmou { namespace_lock(); lock_mount_hash(); - umount_tree(real_mount(mnt), UMOUNT_SYNC); + umount_tree(real_mount(mnt), 0); unlock_mount_hash(); namespace_unlock(); }