All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: linux-fsdevel@vger.kernel.org,
	Andrei Vagin <avagin@virtuozzo.com>,
	Ram Pai <linuxram@us.ibm.com>
Subject: Re: [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts.
Date: Thu, 12 Jan 2017 05:30:20 +0000	[thread overview]
Message-ID: <20170112053020.GS1555@ZenIV.linux.org.uk> (raw)
In-Reply-To: <87inplh8or.fsf_-_@xmission.com>

On Thu, Jan 12, 2017 at 05:18:12AM +1300, Eric W. Biederman wrote:
> 
> When I look at what propagate_mount_busy is trying to do and I look
> at the code closely I discover there is a great disconnect between the
> two.  In the ordinary non-propagation case propagate_mount_busy has
> been verifying that there are no submounts and that there are no
> extraneous references on the mount.
> 
> For mounts that the unmount would propagate to propagate_mount_busy has
> been verifying that there are no extraneous references only if there
> are no submounts.  Which is nonsense.

... because?

> Thefore rework the logic in propgate_mount_busy so that for each
> mount it examines it considers that mount busy if that mount has
> children or if there are extraneous references to that mount.
> 
> While this check was incorrect we could leak mounts instead of simply
> failing umount.

	What do you mean, leak?  We ended up not unmounting them, and they
stayed around until umount of whatever they'd been shadowed by/slipped under
had exposed them and they got explicitly unmounted.

	This is not a leak in a sense of "data structure is unreachable and
will never be freed", unlike the one your previous version had introduced.

Your change might very well be a nicer behaviour - or a DoS in making.
But it really deserves more detailed rationale than that and yes, it
is a user-visible change.  With rather insane userland setups in that
area (*cough* systemd *cough* docker), it's _not_ obviously correct.


  parent reply	other threads:[~2017-01-12  5:30 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-31  4:10 [PATCH] Fix a race in put_mountpoint Krister Johansen
2016-12-31  6:17 ` Al Viro
2017-01-03  0:51   ` Eric W. Biederman
2017-01-03  1:48     ` Al Viro
2017-01-03  3:17       ` Eric W. Biederman
2017-01-03  4:00         ` Al Viro
2017-01-04  3:52           ` Eric W. Biederman
2017-01-04  3:53             ` [PATCH] mnt: Protect the mountpoint hashtable with mount_lock Eric W. Biederman
2017-01-04 21:04               ` [REVIEW][PATCH] mnt: Tuck mounts under others instead of creating shadow/side mounts Eric W. Biederman
2017-01-07  5:06                 ` Al Viro
2017-01-11  0:10                   ` Eric W. Biederman
2017-01-11  4:11                     ` Al Viro
2017-01-11 16:03                       ` Eric W. Biederman
2017-01-11 16:18                         ` [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts Eric W. Biederman
2017-01-11 16:19                           ` [REVIEW][PATCH 2/2] mnt: Tuck mounts under others instead of creating shadow/side mounts Eric W. Biederman
2017-01-12  5:45                             ` Al Viro
2017-01-20  7:20                               ` Eric W. Biederman
2017-01-20  7:26                               ` [PATCH v5] " Eric W. Biederman
2017-01-21  3:58                                 ` Ram Pai
2017-01-21  4:15                                   ` Eric W. Biederman
2017-01-23 19:02                                     ` Ram Pai
2017-01-24  0:16                                       ` Eric W. Biederman
2017-02-03 10:54                                         ` Eric W. Biederman
2017-02-03 17:10                                           ` Ram Pai
2017-02-03 18:26                                             ` Eric W. Biederman
2017-02-03 20:28                                               ` Ram Pai
2017-02-03 20:58                                                 ` Eric W. Biederman
2017-02-06  3:25                                                   ` Andrei Vagin
2017-02-06 21:40                                                     ` Ram Pai
2017-02-07  6:35                                                       ` Andrei Vagin
2017-01-12  5:30                           ` Al Viro [this message]
2017-01-20  7:18                             ` [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts Eric W. Biederman
2017-01-13 20:32                           ` Andrei Vagin
2017-01-18 19:20                             ` Andrei Vagin
2017-01-20 23:18                           ` Ram Pai
2017-01-23  8:15                             ` Eric W. Biederman
2017-01-23 17:04                               ` Ram Pai
2017-01-12  5:03                         ` [REVIEW][PATCH] mnt: Tuck mounts under others instead of creating shadow/side mounts Al Viro
2017-05-14  2:15                 ` Andrei Vagin
2017-05-14  4:05                   ` Eric W. Biederman
2017-05-14  9:26                     ` Eric W. Biederman
2017-05-15 18:27                       ` Andrei Vagin
2017-05-15 19:42                         ` Eric W. Biederman
2017-05-15 20:10                           ` [REVIEW][PATCH] mnt: In umount propagation reparent in a separate pass Eric W. Biederman
2017-05-15 23:12                             ` Andrei Vagin
2017-05-16  5:42                             ` [PATCH] test: check a case when a mount is propagated between exiting mounts Andrei Vagin
2017-05-17  5:54                             ` [REVIEW][PATCH 1/2] mnt: In propgate_umount handle visiting mounts in any order Eric W. Biederman
2017-05-17  5:55                               ` [REVIEW][PATCH 2/2] mnt: Make propagate_umount less slow for overlapping mount propagation trees Eric W. Biederman
2017-05-17 22:48                                 ` Andrei Vagin
2017-05-17 23:26                                   ` Eric W. Biederman
2017-05-18  0:51                                     ` Andrei Vagin
2017-05-24 20:42                               ` [REVIEW][PATCH 1/2] mnt: In propgate_umount handle visiting mounts in any order Ram Pai
2017-05-24 21:54                                 ` Eric W. Biederman
2017-05-24 22:35                                   ` Ram Pai
2017-05-30  6:07                               ` Ram Pai
2017-05-30 15:07                                 ` Eric W. Biederman
2017-06-07  9:54                                   ` Ram Pai
2017-06-07 13:09                                     ` Eric W. Biederman
2017-05-22  8:15                             ` [REVIEW][PATCH] mnt: In umount propagation reparent in a separate pass Ram Pai
2017-05-22 18:33                               ` Eric W. Biederman
2017-05-22 22:34                                 ` Ram Pai
2017-05-23 13:58                                   ` Eric W. Biederman
2017-01-06  7:00               ` [PATCH] mnt: Protect the mountpoint hashtable with mount_lock Krister Johansen

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=20170112053020.GS1555@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=avagin@virtuozzo.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linuxram@us.ibm.com \
    /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.