From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zeniv.linux.org.uk ([195.92.253.2]:59864 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750698AbdALFaW (ORCPT ); Thu, 12 Jan 2017 00:30:22 -0500 Date: Thu, 12 Jan 2017 05:30:20 +0000 From: Al Viro To: "Eric W. Biederman" Cc: linux-fsdevel@vger.kernel.org, Andrei Vagin , Ram Pai Subject: Re: [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts. Message-ID: <20170112053020.GS1555@ZenIV.linux.org.uk> References: <87ful07ryd.fsf@xmission.com> <20170103040052.GB1555@ZenIV.linux.org.uk> <87y3yr32ig.fsf@xmission.com> <87shoz32g8.fsf_-_@xmission.com> <87a8b6r0z5.fsf_-_@xmission.com> <20170107050644.GA12074@ZenIV.linux.org.uk> <87fukqh2we.fsf@xmission.com> <20170111041140.GQ1555@ZenIV.linux.org.uk> <87inplinxd.fsf@xmission.com> <87inplh8or.fsf_-_@xmission.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87inplh8or.fsf_-_@xmission.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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.