On Tue, Apr 29, 2025 at 06:10:54AM +0100, Al Viro wrote: > On Tue, Apr 29, 2025 at 05:03:58AM +0100, Al Viro wrote: > > On Mon, Apr 28, 2025 at 07:53:18PM +0100, Al Viro wrote: > > > > > FWIW, I've a series of cleanups falling out of audit of struct mount > > > handling; it's still growing, but I'll post the stable parts for review > > > tonight or tomorrow... > > > > _Another_ fun one, this time around do_umount(). > > ... and more, from 620c266f3949 "fhandle: relax open_by_handle_at() > permission checks" - just what is protecting has_locked_children() > use there? We are, after all, iterating through ->mnt_mounts - > with no locks whatsoever. Not to mention the fun question regarding Mea culpa. That was clearly an obvious accident. It's a not very subtle oversight. > the result (including the bits sensitive to is_mounted()) remaining > valid by the time you get through exportfs_decode_fh_raw() (and no, > you can't hold any namespace_sem over it - no IO allowed under I know. > that, so we'll need to recheck after that point)... Why bother rechecking? It's literally just to prove that the caller could theoretically get an unobstructed view by shedding mounts if they wanted to. This is an approximation _at best_. The caller obviously cannot switch mount namespace so there's no risk of suddenly a bunch of locked mounts popping up. The only way we could suddenly have a locked mount appear is if someone propagated a mount tree into the callers user+mount namespace from a different user+mount namespace and some mounts were about to expire. And it even has to be an actual tree and not just a single mount. So honestly, the one check is just fine. It's imperfect as it is and it's for the special users that really need this. It's blocked in nearly all container runtimes by default anyway because it can be abused to open any crap on the system. This is literally fringe but needed functionality.
