| Message ID | 165751066658.210556.1326573473015621909.stgit@donald.themaw.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | autofs: fix may_umount_tree() | expand |
On Mon, Jul 11, 2022 at 11:37:46AM +0800, Ian Kent wrote: > +static int do_mount_in_use_check(struct mount *mnt, int cnt) > +{ > + struct mount *topper; > + > + /* Is there exactly one mount on the child that covers > + * it completely? > + */ > + topper = find_topper(mnt); > + if (topper) { > + int topper_cnt = topper->mnt_mounts_cnt + 1; > + > + /* Open file or pwd within singular mount? */ > + if (do_refcount_check(topper, topper_cnt)) > + return 1; Whatever the hell for? umount(2) will be able to slide the underlying mount from under the topper, whatever the refcount of topper might have been.
On 20/7/22 09:54, Al Viro wrote: > On Mon, Jul 11, 2022 at 11:37:46AM +0800, Ian Kent wrote: > >> +static int do_mount_in_use_check(struct mount *mnt, int cnt) >> +{ >> + struct mount *topper; >> + >> + /* Is there exactly one mount on the child that covers >> + * it completely? >> + */ >> + topper = find_topper(mnt); >> + if (topper) { >> + int topper_cnt = topper->mnt_mounts_cnt + 1; >> + >> + /* Open file or pwd within singular mount? */ >> + if (do_refcount_check(topper, topper_cnt)) >> + return 1; > Whatever the hell for? umount(2) will be able to slide the > underlying mount from under the topper, whatever the > refcount of topper might have been. My thinking was that a process could have set a working directory (or opened a descriptor) and some later change to an autofs map resulted in it being mounted on. It's irrelevant now with your suggested simpler approach, ;) Ian
On Wed, Jul 20, 2022 at 10:31:26AM +0800, Ian Kent wrote: > > On 20/7/22 09:54, Al Viro wrote: > > On Mon, Jul 11, 2022 at 11:37:46AM +0800, Ian Kent wrote: > > > > > +static int do_mount_in_use_check(struct mount *mnt, int cnt) > > > +{ > > > + struct mount *topper; > > > + > > > + /* Is there exactly one mount on the child that covers > > > + * it completely? > > > + */ > > > + topper = find_topper(mnt); > > > + if (topper) { > > > + int topper_cnt = topper->mnt_mounts_cnt + 1; > > > + > > > + /* Open file or pwd within singular mount? */ > > > + if (do_refcount_check(topper, topper_cnt)) > > > + return 1; > > Whatever the hell for? umount(2) will be able to slide the > > underlying mount from under the topper, whatever the > > refcount of topper might have been. > > My thinking was that a process could have set a working > > directory (or opened a descriptor) and some later change > > to an autofs map resulted in it being mounted on. It's > > irrelevant now with your suggested simpler approach, ;) No, I mean why bother checking refcount of overmount in the first place? umount(2) will *not* consider it as -EBUSY. On propagation under the full overmount it will quietly remove the thing it's overmounting. If you have overmount victim mountpoint stacked like that, with overmount sitting directly on the root subtree covered by the victim, the only things checked will be * victim itself is not busy * victim has nothing mounted deeper in it. In that case it'll collapse to overmount mountpoint and proceed to take the (now detached) victim out.
diff --git a/fs/pnode.c b/fs/pnode.c index 1106137c747a..e2a906db4324 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -404,6 +404,67 @@ int propagate_mount_busy(struct mount *mnt, int refcnt) return 0; } +static int do_mount_in_use_check(struct mount *mnt, int cnt) +{ + struct mount *topper; + + /* Is there exactly one mount on the child that covers + * it completely? + */ + topper = find_topper(mnt); + if (topper) { + int topper_cnt = topper->mnt_mounts_cnt + 1; + + /* Open file or pwd within singular mount? */ + if (do_refcount_check(topper, topper_cnt)) + return 1; + /* Account for singular mount on parent */ + cnt += 1; + } + + if (do_refcount_check(mnt, cnt)) + return 1; + + return 0; +} + +/* + * Check if the mount tree at 'mnt' is in use or any of its + * propogated mounts are in use. + * @mnt: the mount to be checked + * @adjust: caller holds an additional reference to mount + * Check if mnt or any of its propogated mounts have a reference + * count greater than the minimum reference count (ie. are in use). + */ +int propagate_mount_tree_busy(struct mount *mnt, unsigned int flags) +{ + struct mount *parent = mnt->mnt_parent; + struct mount *m, *child; + unsigned int referenced = flags & TREE_BUSY_REFERENCED; + int cnt; + + /* Check for an elevated refcount on the passed in mount. + * If adjust is true the caller holds a reference to the + * passed in mount. + */ + cnt = mnt->mnt_mounts_cnt + (referenced ? 2 : 1); + if (do_mount_in_use_check(mnt, cnt)) + return 1; + + for (m = propagation_next(parent, parent); m; + m = propagation_next(m, parent)) { + child = __lookup_mnt(&m->mnt, mnt->mnt_mountpoint); + if (!child) + continue; + + cnt = child->mnt_mounts_cnt + 1; + + if (do_mount_in_use_check(child, cnt)) + return 1; + } + return 0; +} + /* * Clear MNT_LOCKED when it can be shown to be safe. * diff --git a/fs/pnode.h b/fs/pnode.h index 988f1aa9b02a..d7b9dddb257b 100644 --- a/fs/pnode.h +++ b/fs/pnode.h @@ -30,6 +30,8 @@ #define CL_COPY_ALL (CL_COPY_UNBINDABLE | CL_COPY_MNT_NS_FILE) +#define TREE_BUSY_REFERENCED 0x01 + static inline void set_mnt_shared(struct mount *mnt) { mnt->mnt.mnt_flags &= ~MNT_SHARED_MASK; @@ -41,6 +43,7 @@ int propagate_mnt(struct mount *, struct mountpoint *, struct mount *, struct hlist_head *); int propagate_umount(struct list_head *); int propagate_mount_busy(struct mount *, int); +int propagate_mount_tree_busy(struct mount *, unsigned int); void propagate_mount_unlock(struct mount *); void mnt_release_group_id(struct mount *); int get_dominating_id(struct mount *mnt, const struct path *root);
Now that child mounts are tracked the expire checks need to be able to use this to check if a mount is in use. Currently when checking a mount for expiration may_umount_tree() checks only if the passed in mount is in use. This leads to false positive callbacks to the automount daemon to umount the mount which fail if any propagated mounts are in use. To avoid these unnecessary callbacks may_umount_tree() needs to check propagated mounts in a similar way to may_umount(). Add a helper that can do this. Signed-off-by: Ian Kent <raven@themaw.net> --- fs/pnode.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/pnode.h | 3 +++ 2 files changed, 64 insertions(+)