Bug 221064 - zfs should not be able to shadow mount on root directory from userspace
Summary: zfs should not be able to shadow mount on root directory from userspace
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-fs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-07-28 08:48 UTC by David NewHamlet
Modified: 2017-09-13 23:12 UTC (History)
5 users (show)

See Also:


Attachments
prototype fix for reference (700 bytes, patch)
2017-07-28 08:48 UTC, David NewHamlet
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David NewHamlet 2017-07-28 08:48:41 UTC
Created attachment 184791 [details]
prototype fix for reference

In a shell of normal system, when import a zpool with a dataset which has a mountpoint set to "/" or zfs set mountpoint of a exist dataset to "/", this dataset will be mounted to "/". As a result, the original filesystem will be shadowed(can not access). To mount a new root filesystem, reboot -r(reroot) should be used.

The attachment is a prototype fix for reference.
Comment 1 Allan Jude freebsd_committer 2017-07-28 15:21:07 UTC
The normal work-around for this is to import with an altroot, usually by specifying 'zpool import -R /mnt poolname' or similar.

This approach might make sense, although the new default in FreeBSD systems created with the installer is for the / partition to be 'canmount=noauto', which causes a different problem, where only the non-root datasets get mounted at import.

I don't know if it makes sense to have a new value for canmount=ifroot or something.
Comment 2 David NewHamlet 2017-07-28 21:48:35 UTC
(In reply to Allan Jude from comment #1)

Thanks for your explaining. Using altroot/canmount=noauto/import -N for import a pool, which has mountpoint=/ with a dataset, is a way to avoid shadown mount. I am wared befor sent this report.

However, never be able to shadown mount rootfs from userspace should be a common rule for all filesystem. This is why I think a work-around is not enough and send a report.
Comment 3 Allan Jude freebsd_committer 2017-07-28 22:46:09 UTC
(In reply to David NewHamlet from comment #2)
Yes, at this point it is a policy decision, and I need to think about it some, and maybe talk to the upstream OpenZFS project about it.
Comment 4 Andriy Gapon freebsd_committer 2017-07-29 07:08:45 UTC
(In reply to Allan Jude from comment #3)
I am not sure that that policy has anything to do with OpenZFS.
Each operating system handles that in its own way.
For example, as far as I know, in illumos you can not mount any filesystem over any non-empty directory.

In FreeBSD we allow mounts anywhere, including root.
And there is nothing ZFS specific about that.
I personally think that allowing to mount over / is more dangerous than useful. But people always find creative uses for every feature and misfeature, so I think that a wider discussion about such a fundamental decision will be required.
Comment 5 Poul-Henning Kamp freebsd_committer 2017-07-29 07:48:11 UTC
Quite the contrary, it is far more useful than dangerous:  It is not
uncommon for embedded systems to boot with a absolutely minimal root
filesystem compiled into the kernel, and /sbin/init in that filesystem
will mount the "real" root over / and exec the "real" /sbin/init.

(And remember: FreeBSD delivers tools, not policies).
Comment 6 David NewHamlet 2017-07-29 08:34:39 UTC
(In reply to Poul-Henning Kamp from comment #5)

kenv vfs.root.mountfrom="second rootfs(zfs:newtank/rootfs)" and reboot -r will trigger kernel to stop all running proccess(include first init) and umount first rootfs. After that, kernel will try to mount second filesystem as rootfs and execute init from second rootfs. 

This might be a better way to handle your case.

This is a useful feature similar to switch_root in Linux world which is very common in embedded linux.

It was introduced by trasz on Sep 18 2015, 5:35 PM.

https://reviews.freebsd.org/D3693
Comment 7 David NewHamlet 2017-07-29 08:38:58 UTC
reboot -r support in kernel:

https://reviews.freebsd.org/D2698
Comment 8 Poul-Henning Kamp freebsd_committer 2017-07-29 09:03:10 UTC
Yes, that's very useful and without dispute a better way to do that.

But we still shouldn't remove a feature people use without due notice.
Comment 9 David NewHamlet 2017-07-30 19:47:14 UTC
(In reply to Poul-Henning Kamp from comment #8)
I agree with you. Before disable rootfs shadow mounting by default, notice should be announced in a period of time (at least 1-2 release cycle).
Comment 10 Ed Maste freebsd_committer 2017-09-13 21:21:13 UTC
(In reply to Poul-Henning Kamp from comment #5)
I agree that using 'mount' to mount a new fs over an existing one is probably intentional, but the issue with ZFS here is that the mount happens as a side effect of 'zpool import' without the user explicitly asking for this.
Comment 11 Allan Jude freebsd_committer 2017-09-13 23:12:37 UTC
(In reply to Ed Maste from comment #10)
Maybe we could create a new mount option, noshadow, and make 'zfs mount -a' and maybe even regular 'mount -a' use it by default, but manual mounts would not.

When noshadow is specified, only mount if the target mountpoint is an empty directory.