From patchwork Thu Apr 7 18:19:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 8775691 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1E7989F3D1 for ; Thu, 7 Apr 2016 18:19:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F23EE20256 for ; Thu, 7 Apr 2016 18:19:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DB17120222 for ; Thu, 7 Apr 2016 18:19:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757299AbcDGSTQ (ORCPT ); Thu, 7 Apr 2016 14:19:16 -0400 Received: from mail-pf0-f171.google.com ([209.85.192.171]:33355 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757020AbcDGSTO (ORCPT ); Thu, 7 Apr 2016 14:19:14 -0400 Received: by mail-pf0-f171.google.com with SMTP id 184so60369072pff.0 for ; Thu, 07 Apr 2016 11:19:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osandov-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:mime-version:content-disposition :user-agent; bh=baeSNHFZvFP0LQxfa7KFWQU6WbPBb3UynWX2IWpYy3A=; b=OTpBPoxdSj1qpelN0ph/qs/C7YaPC//95EePCVAn38pJZTgkMwqgQqXisDuijdfdsE H/7v7Tu05zIHAP3brMEo3h7McqV/BqmjyM4Y4RGKG2cITC28TwZCbvOGWkQjN/tS0LDj m25pjFE22bJ6qwfv3kInlNUm51S4GG3VbDBvBc41TRl5lGSJi4Jr5Gy8AgEBH9d37fQR Up7r7osQc/bTxmJhI1nnflScvFElJCqq0pzsBP4FJ6qz1w5f51K0IP4bskM3eGx+ibZQ 7saUy/kqlMf8N6ax18OGmWTfPIMKXoqxOx5KI37SA5MNndrkDmxCrw1PCanzucUrYep4 16zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=baeSNHFZvFP0LQxfa7KFWQU6WbPBb3UynWX2IWpYy3A=; b=PpiKTheRoR0je2P7OUiZ7wbBGj3yboFN8hnqjBQ+WfphR0BPxWDrzidkltCfNCOAbT 3BnH6vUCKfX36D8iUcZ2c+07YKY0z9KT77Pz1b/TgRopeIKniV5eEPVM09euDOi0yrql XRF340dnD4sjOKjlNgSbcfRl8LiYdSzR3lHrrOcWlxTMQOF5VgpMIR5IMfvqmCZ9GzbO 1gVc6gIHYt+lht1QcX4k/FlVyxc6IldrnE+pgQeYCGxpVmhdM816bsXztApF8VaD7iPV Ja4GQoUw9nuOQZoeBRHrDwcMeN1IHwyudWGzk3z2IiqVnGbmOneu9c4nIoIGRck0wnqz iT1Q== X-Gm-Message-State: AD7BkJIYhf4KTywLYlq1EbYqI3PHAGWBBI707TT1Ws92xPzuiN5FO8BHGPerJrNbZ++t+d3U X-Received: by 10.98.70.27 with SMTP id t27mr6420433pfa.107.1460053153160; Thu, 07 Apr 2016 11:19:13 -0700 (PDT) Received: from vader ([2601:602:8c02:34a0:1a5e:fff:fea7:e0ef]) by smtp.gmail.com with ESMTPSA id by3sm13723761pab.39.2016.04.07.11.19.12 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 07 Apr 2016 11:19:12 -0700 (PDT) Date: Thu, 7 Apr 2016 11:19:11 -0700 From: Omar Sandoval To: Ian Kent Cc: autofs@vger.kernel.org, linux-fsdevel@vger.kernel.org, kernel-team@fb.com Subject: Autofs and mount namespaces Message-ID: <20160407181911.GA9018@vader> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, Ian, I wanted to bring up the issue of autofs and mount namespaces again, which recently resurfaced in the thread here [1]. In that thread, you mentioned that you had some patches to make autofs namespace-aware that you were holding on to. Do you think you could put those up so we can all work out the issues you mentioned? For some background, we've also been bitten by the lack of namespace-awareness in autofs many times. The simple case that breaks can be reproduced as follows, assuming that /mnt/foo is an indirect mount: ls /mnt/foo # do the initial automount unshare -m sleep 10 & # hold the automount in a new namespace umount /mnt/foo # pretend the mount timed out ls /mnt/foo # try to access it again ls: cannot open directory '/mnt/foo': Too many levels of symbolic links For us, the unshare step is actually setting up a container. Our workaround was to make sure that we clean up all of the base system's mounts when setting up the container, but in the general case this is still broken. In this particular case, I believe the problem is that we're using `d_mountpoint()` (or `have_submounts()`) in several places to check whether something is already mounted, and that's not namespace-aware. This hack mitigates the problem I saw above, although it probably ignores edge cases that the old code was handling: ---- ---- Thanks. 1: http://thread.gmane.org/gmane.linux.kernel.autofs/6868/focus=7260 2: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/autofs4/root.c?h=v4.6-rc2#n381 --- Omar -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index 7ab923940d18..a620822342c8 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -378,39 +378,29 @@ static struct vfsmount *autofs4_d_automount(struct path *path) goto done; } - if (!d_mountpoint(dentry)) { - /* - * It's possible that user space hasn't removed directories - * after umounting a rootless multi-mount, although it - * should. For v5 have_submounts() is sufficient to handle - * this because the leaves of the directory tree under the - * mount never trigger mounts themselves (they have an autofs - * trigger mount mounted on them). But v4 pseudo direct mounts - * do need the leaves to trigger mounts. In this case we - * have no choice but to use the list_empty() check and - * require user space behave. - */ - if (sbi->version > 4) { - if (have_submounts(dentry)) { - spin_unlock(&sbi->fs_lock); - goto done; - } - } else { - if (!simple_empty(dentry)) { - spin_unlock(&sbi->fs_lock); - goto done; - } - } - ino->flags |= AUTOFS_INF_PENDING; - spin_unlock(&sbi->fs_lock); - status = autofs4_mount_wait(dentry, 0); - spin_lock(&sbi->fs_lock); - ino->flags &= ~AUTOFS_INF_PENDING; - if (status) { + if (d_mountpoint(dentry)) { + /* Make sure this is a mountpoint in this namespace. */ + struct vfsmount *mounted = lookup_mnt(path); + if (mounted) { + mntput(mounted); spin_unlock(&sbi->fs_lock); - return ERR_PTR(status); + goto done; } } + + if (!simple_empty(dentry)) { + spin_unlock(&sbi->fs_lock); + goto done; + } + ino->flags |= AUTOFS_INF_PENDING; + spin_unlock(&sbi->fs_lock); + status = autofs4_mount_wait(dentry, 0); + spin_lock(&sbi->fs_lock); + ino->flags &= ~AUTOFS_INF_PENDING; + if (status) { + spin_unlock(&sbi->fs_lock); + return ERR_PTR(status); + } spin_unlock(&sbi->fs_lock); done: /* Mount succeeded, check if we ended up with a new dentry */ diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c index 0146d911f468..b4e901d2aa6b 100644 --- a/fs/autofs4/waitq.c +++ b/fs/autofs4/waitq.c @@ -332,8 +332,6 @@ static int validate_request(struct autofs_wait_queue **wait, dentry = new; } } - if (have_submounts(dentry)) - valid = 0; if (new) dput(new); diff --git a/fs/internal.h b/fs/internal.h index b71deeecea17..c26ba59eec1b 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -58,7 +58,6 @@ extern int vfs_path_lookup(struct dentry *, struct vfsmount *, extern void *copy_mount_options(const void __user *); extern char *copy_mount_string(const void __user *); -extern struct vfsmount *lookup_mnt(struct path *); extern int finish_automount(struct vfsmount *, struct path *); extern int sb_prepare_remount_readonly(struct super_block *); diff --git a/include/linux/mount.h b/include/linux/mount.h index f822c3c11377..ff971448152f 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -72,6 +72,7 @@ struct vfsmount { struct file; /* forward dec */ struct path; +extern struct vfsmount *lookup_mnt(struct path *); extern int mnt_want_write(struct vfsmount *mnt); extern int mnt_want_write_file(struct file *file); extern int mnt_clone_write(struct vfsmount *mnt);