[PATCH review 0/7] Bind mount escape fixes
torvalds at linux-foundation.org
Sat Aug 15 19:36:41 UTC 2015
On Sat, Aug 15, 2015 at 11:35 AM, Eric W. Biederman
<ebiederm at xmission.com> wrote:
> The implementation change this round is I have dropped my patch cleaning
> up d_splice_alias. Al Viro found a race that makes the technique I was
> using fundamentally racy. I now have d_splice_alias taking mount_lock
> around rename_lock. Since I don't have to sleep in d_splice_alias
> change is minimal and sufficient for this purpose.
Quite frankly, I hate this series.
Not all of it. Patches 1,2 and 5 look fine to me. But 3,4,6 and 7 I'm
not happy with. Particularly patch 6.
It just smells like a bad hack to me. My gut feel says that it's all
wrong. It doesn't feel clean or right.
I'd much rather make ".." handling more expensive than add and
maintain that MNT_DIR_ESCAPED flag. My gut feel is that yes, we
should look seriously at making ".." much smarter (so I don't object
to the concept of patch 7/7 at all), but I think we should strive to
look at that ".." handling *without* adding the crufty odd special
bind mount crud.
Put another way: the whole "escape bind mount" thing is not at all a
new issue, it smells very much like the very traditional Unix "escape
chroot" thing. And I detest how this adds magic rules for bind mounts,
when it feels like a much more generic issue.
Ok, so I haven't really thought deeply about this, this literally is
just a "gut feel" kind of thing.
Can we really not validate ".." some clever way _without_ adding all
those "mount escape" flags? And by "clever" I potentially mean "not
clever" and in fact just fairly brute force. I'd almost prefer to just
walk the parent chains all the way to the root and validate the ".."
More information about the Containers