From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1763086AbcIOAct (ORCPT ); Wed, 14 Sep 2016 20:32:49 -0400 Received: from mail-pa0-f53.google.com ([209.85.220.53]:34161 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752232AbcIOAcq (ORCPT ); Wed, 14 Sep 2016 20:32:46 -0400 Date: Wed, 14 Sep 2016 17:32:43 -0700 From: Omar Sandoval To: Ian Kent Cc: "Eric W. Biederman" , Andrew Morton , autofs mailing list , Kernel Mailing List , Al Viro , linux-fsdevel , kernel-team@fb.com Subject: Re: [PATCH 3/4] autofs - make mountpoint checks namespace aware Message-ID: <20160915003243.GA16372@vader> References: <20160914061434.24714.490.stgit@pluto.themaw.net> <20160914061445.24714.68331.stgit@pluto.themaw.net> <87zina9ys3.fsf@x220.int.ebiederm.org> <1473898163.3205.32.camel@themaw.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1473898163.3205.32.camel@themaw.net> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 15, 2016 at 08:09:23AM +0800, Ian Kent wrote: > On Wed, 2016-09-14 at 12:28 -0500, Eric W. Biederman wrote: > > Ian Kent writes: > > > > > If an automount mount is clone(2)ed into a file system that is > > > propagation private, when it later expires in the originating > > > namespace subsequent calls to autofs ->d_automount() for that > > > dentry in the original namespace will return ELOOP until the > > > mount is manually umounted in the cloned namespace. > > > > > > In the same way, if an autofs mount is triggered by automount(8) > > > running within a container the dentry will be seen as mounted in > > > the root init namespace and calls to ->d_automount() in that namespace > > > will return ELOOP until the mount is umounted within the container. > > > > > > Also, have_submounts() can return an incorect result when a mount > > > exists in a namespace other than the one being checked. > > > > Overall this appears to be a fairly reasonable set of changes. It does > > increase the expense when an actual mount point is encountered, but if > > these are the desired some increase in cost when a dentry is a > > mountpoint is unavoidable. > > > > May I ask the motiviation for this set of changes? Reading through the > > changes I don't grasp why we want to change the behavior of autofs. > > What problem is being solved? What are the benefits? > > LOL, it's all too easy for me to give a patch description that I think explains > a problem I need to solve without realizing it isn't clear to others what the > problem is, sorry about that. > > For quite a while now, and not that frequently but consistently, I've been > getting reports of people using autofs getting ELOOP errors and not being able > to mount automounts. > > This has been due to the cloning of autofs file systems (that have active > automounts at the time of the clone) by other systems. > > An unshare, as one example, can easily result in the cloning of an autofs file > system that has active mounts which shows this problem. > > Once an active mount that has been cloned is expired in the namespace that > performed the unshare it can't be (auto)mounted again in the the originating > namespace because the mounted check in the autofs module will think it is > already mounted. > > I'm not sure this is a clear description either, hopefully it is enough to > demonstrate the type of problem I'm typing to solve. Yup, at Facebook we've been hitting this issue for years. Our container setup doesn't clean up the base system's mounts after the unshare(CLONE_NEWNS) and before the chroot(), so we very frequently see the base system's autofs mounts get broken with ELOOP. The solution there might be to fix our container setup, but I think it's still a kernel bug, as it breaks the isolation between namespaces. Ian, I'm going to test these patches, thanks for sending them out. -- Omar