All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
To: David Howells <dhowells@redhat.com>, viro@zeniv.linux.org.uk
Cc: torvalds@linux-foundation.org, ebiederm@xmission.com,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	mszeredi@redhat.com
Subject: Re: [PATCH 03/34] teach move_mount(2) to work with OPEN_TREE_CLONE [ver #12]
Date: Fri, 5 Oct 2018 19:24:37 +0100	[thread overview]
Message-ID: <de050902-f3d0-b2db-2627-983a36c87b3c@gmail.com> (raw)
In-Reply-To: <153754743491.17872.12115848333103740766.stgit@warthog.procyon.org.uk>

On 21/09/2018 17:30, David Howells wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
> attached by move_mount(2).
>
> If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
> not detached anymore, it won't be dissolved.  move_mount(2) is adjusted
> to handle detached source.
>
> That gives us equivalents of mount --bind and mount --rbind.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
>   fs/namespace.c |   26 ++++++++++++++++++++------
>   1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index dd38141b1723..caf5c55ef555 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1785,8 +1785,10 @@ void dissolve_on_fput(struct vfsmount *mnt)
>   {
>   	namespace_lock();
>   	lock_mount_hash();
> -	mntget(mnt);
> -	umount_tree(real_mount(mnt), UMOUNT_CONNECTED);
> +	if (!real_mount(mnt)->mnt_ns) {
> +		mntget(mnt);
> +		umount_tree(real_mount(mnt), UMOUNT_CONNECTED);
> +	}
>   	unlock_mount_hash();
>   	namespace_unlock();
>   }
> @@ -2393,6 +2395,7 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
>   	struct mount *old;
>   	struct mountpoint *mp;
>   	int err;
> +	bool attached;
>   
>   	mp = lock_mount(new_path);
>   	err = PTR_ERR(mp);
> @@ -2403,10 +2406,19 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
>   	p = real_mount(new_path->mnt);
>   
>   	err = -EINVAL;
> -	if (!check_mnt(p) || !check_mnt(old))
> +	/* The mountpoint must be in our namespace. */
> +	if (!check_mnt(p))
> +		goto out1;
> +	/* The thing moved should be either ours or completely unattached. */
> +	if (old->mnt_ns && !check_mnt(old))
>   		goto out1;
>   
> -	if (!mnt_has_parent(old))
> +	attached = mnt_has_parent(old);
> +	/*
> +	 * We need to allow open_tree(OPEN_TREE_CLONE) followed by
> +	 * move_mount(), but mustn't allow "/" to be moved.
> +	 */
> +	if (old->mnt_ns && !attached)
>   		goto out1;
>   
>   	if (old->mnt.mnt_flags & MNT_LOCKED)

Hi

I replied last time to wonder about the MNT_UMOUNT mnt_flag. So I've 
tested it now :-), on David's current tree (commit 5581f4935add).

The modified do_move_mount() allows re-attaching something that was 
lazy-unmounted. But the lazy unmount sets MNT_UMOUNT. And this flag is 
not cleared when the mount is re-attached.

I wasn't sure what effect this would have. Luckily it showed up straight 
away, when I tried to unmount again. It causes a soft lockup.

Debug printk:

diff --git a/fs/namespace.c b/fs/namespace.c
index 4dfe7e23b7ee..ac8de9191cfe 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2472,6 +2472,10 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
  	if (old->mnt.mnt_flags & MNT_LOCKED)
  		goto out1;
  
+	pr_info("mnt_flags=%x umount=%x\n",
+	        (unsigned) old->mnt.mnt_flags,
+	        (unsigned) !!(old->mnt.mnt_flags & MNT_UMOUNT);
+
  	if (old_path->dentry != old_path->mnt->mnt_root)
  		goto out1;

Testing:

# mount -ttmpfs tmp /mnt
# cd /mnt
# umount .
umount: /mnt: target is busy.
# umount -l .
# mount --move . /mnt
[  577.773804] mnt_flags=8000020 umount=1

Double-check the flags after the mount is re-attached:

# mount --move . /mnt
[  610.891311] mnt_flags=8000020 umount=1
mount: /mnt: mount(2) system call failed: Too many levels of symbolic links.

The bug:

# cd
# umount /mnt
[  656.229099] watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [umount:1457]
[  656.230231] Modules linked in: xt_CHECKSUM(E) ipt_MASQUERADE(E) tun(E) bridge(E) stp(E) llc(E) ip6t_rpfilter(E) ip6t_REJECT(E) nf_reject_ipv6(E) xt_conntrack(E) devlink(E) ip6table_nat(E) nf_nat_ipv6(E) ip6table_mangle(E) ip6table_raw(E) ip6table_security(E) iptable_nat(E) nf_nat_ipv4(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) libcrc32c(E) nf_defrag_ipv4(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ip6table_filter(E) ip6_tables(E) snd_hda_codec_generic(E) snd_hda_intel(E) snd_hda_codec(E) snd_hwdep(E) snd_hda_core(E) snd_seq(E) snd_seq_device(E) snd_pcm(E) joydev(E) crc32_pclmul(E) snd_timer(E) snd(E) ghash_clmulni_intel(E) crct10dif_pclmul(E) virtio_balloon(E) soundcore(E) serio_raw(E) crc32c_intel(E) qxl(E) virtio_console(E) virtio_net(E) net_failover(E) failover(E) drm_kms_helper(E)
[  656.242150]  ttm(E) drm(E) qemu_fw_cfg(E) pata_acpi(E) ata_generic(E)
[  656.243333] CPU: 0 PID: 1457 Comm: umount Tainted: G            E     4.19.0-rc3+ #7
[  656.244767] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180531_142017-buildhw-08.phx2.fedoraproject.org-1.fc28 04/01/2014
[  656.247038] RIP: 0010:pin_kill+0x128/0x140
[  656.247789] Code: f2 5a 00 48 8b 44 24 20 48 39 c5 0f 84 6f ff ff ff 48 89 df e8 e9 4a 5b 00 8b 43 18 85 c0 7e b3 c6 03 00 fb 66 0f 1f 44 00 00 <e9> 51 ff ff ff e8 be 11 dd ff 0f 1f 40 00 66 2e 0f 1f 84 00 00 00
[  656.250738] RSP: 0018:ffffa58040f93e30 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff13
[  656.251984] RAX: 0000000000000000 RBX: ffff971a6b16dc30 RCX: dead000000000200
[  656.253183] RDX: 0000000000000001 RSI: ffffa58040f93dd0 RDI: ffff971a6b16dc30
[  656.254484] RBP: ffffa58040f93e50 R08: 000000000000067d R09: 000000000000067d
[  656.255838] R10: 0000000000000000 R11: 0000000000000000 R12: ffff971a6b2b1800
[  656.257181] R13: ffff971a6b16db88 R14: 0000000000000000 R15: ffff971a6b16db50
[  656.258530] FS:  00007fc7bac88fc0(0000) GS:ffff971ad9600000(0000) knlGS:0000000000000000
[  656.260079] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  656.261165] CR2: 00007fc7ba8704c7 CR3: 000000002d22c001 CR4: 00000000003606f0
[  656.262506] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  656.263690] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  656.265329] Call Trace:
[  656.267958]  ? finish_wait+0x80/0x80
[  656.269083]  group_pin_kill+0x1a/0x30
[  656.269989]  namespace_unlock+0x6f/0x80
[  656.270652]  ksys_umount+0x220/0x420
[  656.271393]  __x64_sys_umount+0x12/0x20
[  656.272249]  do_syscall_64+0x5b/0x160
[  656.272988]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  656.273942] RIP: 0033:0x7fc7b9cd9117
[  656.274630] Code: ed 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 59 ed 2b 00 f7 d8 64 89 01 48
[  656.278886] RSP: 002b:00007ffe0a557498 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[  656.281518] RAX: ffffffffffffffda RBX: 0000556bab8bd420 RCX: 00007fc7b9cd9117
[  656.283138] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000556bab8bd600
[  656.284757] RBP: 0000000000000000 R08: 0000556bab8bd620 R09: 00007ffe0a555d00
[  656.286367] R10: 0000000000000000 R11: 0000000000000246 R12: 0000556bab8bd600
[  656.288408] R13: 00007fc7baa7f1a4 R14: 0000000000000000 R15: 00007ffe0a557708


  reply	other threads:[~2018-10-05 18:24 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-21 16:30 [PATCH 00/34] VFS: Introduce filesystem context [ver #12] David Howells
2018-09-21 16:30 ` David Howells
2018-09-21 16:30 ` David Howells
2018-09-21 16:30 ` [PATCH 01/34] vfs: syscall: Add open_tree(2) to reference or clone a mount " David Howells
2018-10-21 16:41   ` Eric W. Biederman
2018-09-21 16:30 ` [PATCH 02/34] vfs: syscall: Add move_mount(2) to move mounts around " David Howells
2018-09-21 16:30 ` [PATCH 03/34] teach move_mount(2) to work with OPEN_TREE_CLONE " David Howells
2018-10-05 18:24   ` Alan Jenkins [this message]
2018-10-07 10:48     ` Alan Jenkins
2018-10-07 19:20       ` Alan Jenkins
2018-10-10 12:36       ` David Howells
2018-10-12 14:22         ` Alan Jenkins
2018-10-12 14:22           ` Alan Jenkins
2018-10-12 14:54         ` David Howells
2018-10-12 14:57           ` Alan Jenkins
2018-10-11  9:17       ` David Howells
2018-10-11 11:48         ` Alan Jenkins
2018-10-11 13:10         ` David Howells
2018-10-11 12:14       ` David Howells
2018-10-11 12:23         ` Alan Jenkins
2018-10-11 15:33       ` David Howells
2018-10-11 18:38         ` Eric W. Biederman
2018-10-11 20:17         ` David Howells
2018-10-13  6:06           ` Al Viro
2018-10-17 17:45       ` Alan Jenkins
2018-10-18 20:09     ` David Howells
2018-10-18 20:58     ` David Howells
2018-10-19 11:57     ` David Howells
2018-10-19 13:37     ` David Howells
2018-10-19 17:35       ` Alan Jenkins
2018-10-19 21:35       ` David Howells
2018-10-19 21:40       ` David Howells
2018-10-19 22:36       ` David Howells
2018-10-20  5:25         ` Al Viro
2018-10-20 11:06         ` Alan Jenkins
2018-10-20 11:48           ` Al Viro
2018-10-20 11:48             ` Al Viro
2018-10-20 12:26             ` Al Viro
2018-10-21  0:40         ` David Howells
2018-10-10 11:56   ` David Howells
2018-10-10 12:31   ` David Howells
2018-10-10 12:39     ` Alan Jenkins
2018-10-10 12:50   ` David Howells
2018-10-10 13:02   ` David Howells
2018-10-10 13:06     ` Alan Jenkins
2018-10-21 16:57   ` Eric W. Biederman
2018-10-23 11:19   ` Alan Jenkins
2018-10-23 16:22     ` Al Viro
2018-09-21 16:30 ` [PATCH 04/34] vfs: Suppress MS_* flag defs within the kernel unless explicitly enabled " David Howells
2018-09-21 16:30 ` [PATCH 05/34] vfs: Introduce the basic header for the new mount API's filesystem context " David Howells
2018-09-21 16:30 ` [PATCH 06/34] vfs: Introduce logging functions " David Howells
2018-09-21 16:31 ` [PATCH 07/34] vfs: Add configuration parser helpers " David Howells
2019-03-14  7:46   ` Geert Uytterhoeven
2019-03-14 10:27   ` David Howells
2019-03-14 10:49     ` Geert Uytterhoeven
2018-09-21 16:31 ` [PATCH 08/34] vfs: Add LSM hooks for the new mount API " David Howells
2018-09-21 16:31   ` David Howells
2018-09-21 16:31 ` [PATCH 09/34] vfs: Put security flags into the fs_context struct " David Howells
2018-09-21 16:31 ` [PATCH 10/34] selinux: Implement the new mount API LSM hooks " David Howells
2018-09-21 16:31   ` David Howells
2018-09-21 16:31 ` [PATCH 11/34] smack: Implement filesystem context security " David Howells
2018-09-21 16:31   ` David Howells
2018-09-21 16:31 ` [PATCH 12/34] apparmor: Implement security hooks for the new mount API " David Howells
2018-09-21 16:31   ` David Howells
2018-09-21 16:31 ` [PATCH 13/34] tomoyo: " David Howells
2018-09-21 16:31   ` David Howells
2018-09-21 16:32 ` [PATCH 14/34] vfs: Separate changing mount flags full remount " David Howells
2018-09-21 16:32 ` [PATCH 15/34] vfs: Implement a filesystem superblock creation/configuration context " David Howells
2018-09-21 16:32 ` [PATCH 16/34] vfs: Remove unused code after filesystem context changes " David Howells
2018-09-21 16:32 ` [PATCH 17/34] procfs: Move proc_fill_super() to fs/proc/root.c " David Howells
2018-09-21 16:32 ` [PATCH 18/34] proc: Add fs_context support to procfs " David Howells
2018-09-21 16:32 ` [PATCH 19/34] ipc: Convert mqueue fs to fs_context " David Howells
2018-09-21 16:32 ` [PATCH 20/34] cpuset: Use " David Howells
2018-09-21 16:33 ` [PATCH 21/34] kernfs, sysfs, cgroup, intel_rdt: Support " David Howells
2018-11-19  4:23   ` Andrei Vagin
2018-12-06 17:08     ` Andrei Vagin
2018-09-21 16:33 ` [PATCH 22/34] hugetlbfs: Convert to " David Howells
2018-09-21 16:33 ` [PATCH 23/34] vfs: Remove kern_mount_data() " David Howells
2018-09-21 16:33 ` [PATCH 24/34] vfs: Provide documentation for new mount API " David Howells
2018-09-21 16:33 ` [PATCH 25/34] Make anon_inodes unconditional " David Howells
2018-09-21 16:33 ` [PATCH 26/34] vfs: syscall: Add fsopen() to prepare for superblock creation " David Howells
2018-09-21 16:33 ` [PATCH 27/34] vfs: Implement logging through fs_context " David Howells
2018-09-21 16:33 ` [PATCH 28/34] vfs: Add some logging to the core users of the fs_context log " David Howells
2018-09-21 16:34 ` [PATCH 29/34] vfs: syscall: Add fsconfig() for configuring and managing a context " David Howells
2018-09-21 16:34 ` [PATCH 30/34] vfs: syscall: Add fsmount() to create a mount for a superblock " David Howells
2018-09-21 16:34 ` [PATCH 31/34] vfs: syscall: Add fspick() to select a superblock for reconfiguration " David Howells
2018-10-12 14:49   ` Alan Jenkins
2018-10-13  6:11     ` Al Viro
2018-10-13  6:11       ` Al Viro
2018-10-13  9:45       ` Alan Jenkins
2018-10-13 23:04         ` Andy Lutomirski
2018-10-17 13:15       ` David Howells
2018-10-17 13:20       ` David Howells
2018-10-17 13:20         ` David Howells
2018-10-17 14:31         ` Alan Jenkins
2018-10-17 14:35           ` Eric W. Biederman
2018-10-17 14:55             ` Alan Jenkins
2018-10-17 15:24           ` David Howells
2018-10-17 15:24             ` David Howells
2018-10-17 15:38             ` Eric W. Biederman
2018-10-17 15:45         ` David Howells
2018-10-17 17:41           ` Alan Jenkins
2018-10-17 21:20           ` David Howells
2018-10-17 22:13           ` Alan Jenkins
2018-09-21 16:34 ` [PATCH 32/34] afs: Add fs_context support " David Howells
2018-09-21 16:34 ` [PATCH 33/34] afs: Use fs_context to pass parameters over automount " David Howells
2018-09-21 16:34 ` [PATCH 34/34] vfs: Add a sample program for the new mount API " David Howells
2018-12-17 14:12   ` Anders Roxell
2018-12-20 16:36   ` David Howells
2018-10-04 18:37 ` [PATCH 00/34] VFS: Introduce filesystem context " Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=de050902-f3d0-b2db-2627-983a36c87b3c@gmail.com \
    --to=alan.christopher.jenkins@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.