diff mbox series

[2/3] vfs: add propagate_mount_tree_busy() helper

Message ID 165751066658.210556.1326573473015621909.stgit@donald.themaw.net (mailing list archive)
State New
Headers show
Series autofs: fix may_umount_tree() | expand

Commit Message

Ian Kent July 11, 2022, 3:37 a.m. UTC
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(+)

Comments

Al Viro July 20, 2022, 1:54 a.m. UTC | #1
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.
Ian Kent July 20, 2022, 2:31 a.m. UTC | #2
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
Al Viro July 20, 2022, 2:39 a.m. UTC | #3
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 mbox series

Patch

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);