From: ebiederm@xmission.com (Eric W. Biederman)
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
Andy Lutomirski <luto@amacapital.net>,
"Serge E. Hallyn" <serge@hallyn.com>,
Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
Kernel Mailing List <linux-kernel@vger.kernel.org>,
Rob Landley <rob@landley.net>,
Linus Torvalds <torvalds@linux-foundation.org>,
Matthias Schniedermeyer <ms@citd.de>,
Linux Containers <containers@lists.linux-foundation.org>
Subject: Re: [REVIEW][PATCH 1/4] vfs: Don't allow overwriting mounts in the current mount namespace
Date: Fri, 08 Nov 2013 12:51:52 -0800 [thread overview]
Message-ID: <87bo1u8vmf.fsf@xmission.com> (raw)
In-Reply-To: <20131103035406.GA8537@ZenIV.linux.org.uk> (Al Viro's message of "Sun, 3 Nov 2013 03:54:06 +0000")
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Tue, Oct 15, 2013 at 01:16:48PM -0700, Eric W. Biederman wrote:
>
>> int vfs_rmdir(struct inode *dir, struct dentry *dentry)
>> {
>> int error = may_delete(dir, dentry, 1);
>> @@ -3622,6 +3636,9 @@ retry:
>> error = -ENOENT;
>> goto exit3;
>> }
>> + error = -EBUSY;
>> + if (covered(nd.path.mnt, dentry))
>> + goto exit3;
>
> Ugh... And it's not racy because of...? IOW, what's to keep the return
> value of covered() from getting obsolete just as it's being calculated,
> let alone returned?
The return value of d_mountpoint can be obsolete as soon as it returns
as well, so I don't see this as being significantly different.
I would like to say that any changes introduced here do not matter
because all of this is just to keep a semblance of the old semantics.
Unfortunately for me part of keeping that semblance is as much as is
reasonable preserving the existing race guarantees.
In 3.12 we create a mount with:
- The dentry->d_inode mutex held.
- The namespace_sem held.
In 3.12 we remove a mount with just the namespace_sem held.
I call covered in: do_rmdir, do_unlinkat, and renameat.
In 3.12 vfs_rmdir checks d_mountpoint with the
dentry->d_inode->i_mutex and
dentry->d_parent->d_inode->i_mutex held.
In 3.12 vfs_unlink checks d_mountpoint with the
dentry->d_inode->i_mutex and
dentry->d_parent->d_inode->i_mutex hel.d
In 3.12 vfs_rename_dir and vfs_rename_other checks d_mountpint with the
target->i_mutex, new_dir->i_mutex, and old_dir->i_mutex held.
Therefore the guarantees in 3.12 are:
- unlink versus mount races are prevented by the
dentry->d_inode->i_mutex of the dentry being removed.
- unlink versus umount races are uninteresting.
- mount versus rename races in testing of d_mountpoint are ignored.
- umount versus rename races in testing of d_mountpoint are ignored.
So comparing this to how I have implemented covered the test is at a
slightly different location in the call path so there may be a slightly
larger race in rename.
For unlink there is a race where the mount could happen after testing
covered. Then the unlink happens. Then we remove the mount with
detach_mounts.
In the context of the symlink attacks against umounting of fuse I don't
see a difference.
In the only case where there is a new race (unlink versus mount) I see
a narrow window where new behavior will happen the unlink will win and
we unmount the filesystem. So there is a vary narrow window in which
we might have a stale entry in /etc/mtab.
So after all of that analysis I don't think we care. If we do care with
a little more work we can pass the mountpoint down and test covered with
dentry->d_inode->i_mutex held, where we test d_mountpoint in 3.12 today.
Eric
next prev parent reply other threads:[~2013-11-08 20:52 UTC|newest]
Thread overview: 165+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-14 17:42 DoS with unprivileged mounts Miklos Szeredi
2013-08-14 19:26 ` Andy Lutomirski
2013-08-14 19:53 ` Eric W. Biederman
2013-08-14 20:25 ` Andy Lutomirski
2013-08-14 21:54 ` Eric W. Biederman
2013-08-14 19:32 ` Eric W. Biederman
2013-08-15 4:59 ` Miklos Szeredi
2013-08-15 6:45 ` Eric W. Biederman
2013-08-15 6:52 ` Andy Lutomirski
2013-08-15 7:55 ` Eric W. Biederman
2013-08-15 9:29 ` Miklos Szeredi
2013-10-04 22:41 ` [RFC][PATCH 0/3] vfs: Detach mounts on unlink Eric W. Biederman
2013-10-04 22:42 ` [RFC][PATCH 1/3] vfs: Keep a list of mounts on a mount point Eric W. Biederman
2013-10-08 13:44 ` Miklos Szeredi
2013-10-08 20:31 ` Eric W. Biederman
2013-10-04 22:43 ` [RFC][PATCH 2/3] vfs: Add a function to lazily unmount all mounts from any dentry Eric W. Biederman
2013-10-07 4:22 ` Serge E. Hallyn
2013-10-07 6:15 ` Eric W. Biederman
2013-10-08 13:56 ` Miklos Szeredi
2013-10-08 20:32 ` Eric W. Biederman
2013-10-04 22:43 ` [RFC][PATCH 3/3] vfs: Lazily remove mounts on unlinked files and directories Eric W. Biederman
2013-10-08 15:50 ` Miklos Szeredi
2013-10-08 21:47 ` Eric W. Biederman
2013-10-09 19:12 ` Grrrr fusermount Eric W. Biederman
2013-10-09 20:09 ` Andy Lutomirski
2013-10-09 23:35 ` Eric W. Biederman
2013-10-04 23:20 ` [RFC][PATCH 0/3] vfs: Detach mounts on unlink Linus Torvalds
2013-10-05 0:03 ` Eric W. Biederman
2013-10-05 1:57 ` Eric W. Biederman
2013-10-05 23:42 ` Rob Landley
2013-10-06 0:44 ` Eric W. Biederman
2013-10-08 8:03 ` Karel Zak
2013-10-10 6:46 ` Rob Landley
2013-10-05 2:34 ` [RFC][PATCH 4/3] vfs: Allow rmdir to remove mounts in all but the current mount namespace Eric W. Biederman
2013-10-05 15:44 ` Serge E. Hallyn
2013-10-07 4:39 ` Serge E. Hallyn
2013-10-07 6:55 ` Eric W. Biederman
2013-10-07 16:53 ` Andy Lutomirski
2013-10-07 22:25 ` Eric W. Biederman
2013-10-08 16:06 ` Miklos Szeredi
2013-10-08 16:06 ` Andy Lutomirski
2013-10-08 16:11 ` Miklos Szeredi
2013-10-08 20:50 ` Eric W. Biederman
2013-10-10 10:02 ` Miklos Szeredi
2013-10-10 11:43 ` Eric W. Biederman
2013-10-10 11:57 ` Miklos Szeredi
2013-10-12 1:04 ` Eric W. Biederman
2013-10-12 1:39 ` Eric W. Biederman
2013-10-15 20:15 ` [REVIEW][PATCH 0/4] vfs: Detach mounts on unlink Eric W. Biederman
2013-10-15 20:16 ` [REVIEW][PATCH 1/4] vfs: Don't allow overwriting mounts in the current mount namespace Eric W. Biederman
2013-10-22 19:04 ` Serge E. Hallyn
2013-11-03 3:54 ` Al Viro
2013-11-08 20:51 ` Eric W. Biederman [this message]
2013-11-08 21:35 ` Al Viro
2013-11-08 22:17 ` Eric W. Biederman
2013-11-09 8:49 ` Christoph Hellwig
2013-11-21 20:58 ` Eric W. Biederman
2013-11-21 20:49 ` Eric W. Biederman
2013-10-15 20:17 ` [REVIEW][PATCH 2/4] vfs: Keep a list of mounts on a mount point Eric W. Biederman
2013-10-22 19:06 ` Serge E. Hallyn
2013-10-15 20:17 ` [REVIEW][PATCH 3/4] vfs: Add a function to lazily unmount all mounts from any dentry. v3 Eric W. Biederman
2013-10-22 19:08 ` Serge E. Hallyn
2013-10-15 20:18 ` [REVIEW][PATCH 4/4] vfs: Lazily remove mounts on unlinked files and directories. v2 Eric W. Biederman
2013-10-22 19:13 ` Serge E. Hallyn
2013-10-08 10:42 ` [RFC][PATCH 4/3] vfs: Allow rmdir to remove mounts in all but the current mount namespace Matthias Schniedermeyer
2013-10-08 13:13 ` Eric W. Biederman
2013-10-05 23:07 ` [RFC][PATCH 0/3] vfs: Detach mounts on unlink Rob Landley
2013-10-05 23:17 ` Linus Torvalds
2013-10-05 23:22 ` Linus Torvalds
2013-10-06 0:18 ` Rob Landley
2013-10-06 0:37 ` Linus Torvalds
2013-10-05 23:24 ` Al Viro
2013-10-06 0:22 ` Rob Landley
2013-10-06 0:12 ` Rob Landley
2013-10-05 23:19 ` Al Viro
2013-10-06 0:13 ` Rob Landley
2014-02-15 21:34 ` [PATCH 0/11] Detaching " Eric W. Biederman
2014-02-15 21:35 ` [PATCH 01/11] vfs: Document the effect of d_revalidate on d_find_alias Eric W. Biederman
2014-02-15 21:36 ` [PATCH 02/11] vfs: More precise tests in d_invalidate Eric W. Biederman
2014-02-15 22:51 ` Linus Torvalds
2014-02-15 22:59 ` Linus Torvalds
2014-02-15 23:23 ` Eric W. Biederman
2014-02-15 23:39 ` Eric W. Biederman
2014-02-16 0:03 ` Linus Torvalds
2014-02-16 1:22 ` Eric W. Biederman
2014-02-15 21:36 ` [PATCH 03/11] vfs: Don't allow overwriting mounts in the current mount namespace Eric W. Biederman
2014-02-18 17:12 ` Miklos Szeredi
2014-02-18 21:28 ` Eric W. Biederman
2014-02-18 22:20 ` Linus Torvalds
2014-02-19 2:23 ` Stephen Rothwell
2014-02-24 23:43 ` Eric W. Biederman
2014-02-15 21:37 ` [PATCH 04/11] vfs: Keep a list of mounts on a mount point Eric W. Biederman
2014-02-15 21:37 ` [PATCH 05/11] vfs: Add a function to lazily unmount all mounts from any dentry Eric W. Biederman
2014-02-16 1:50 ` Eric W. Biederman
2014-02-18 17:17 ` Miklos Szeredi
2014-02-25 3:24 ` Eric W. Biederman
2014-02-15 21:38 ` [PATCH 06/11] vfs: Lazily remove mounts on unlinked files and directories Eric W. Biederman
2014-02-15 21:38 ` [PATCH 07/11] vfs: Remove unnecessary calls of check_submounts_and_drop Eric W. Biederman
2014-02-15 21:39 ` [PATCH 08/11] vfs: Merge check_submounts_and_drop and d_invalidate Eric W. Biederman
2014-02-18 17:40 ` Miklos Szeredi
2014-02-25 0:01 ` Eric W. Biederman
2014-02-25 15:13 ` J. Bruce Fields
2014-02-25 22:03 ` Eric W. Biederman
2014-02-26 19:37 ` J. Bruce Fields
2014-02-27 2:05 ` Eric W. Biederman
2014-02-15 21:39 ` [PATCH 09/11] vfs: Make d_invalidate return void Eric W. Biederman
2014-02-15 21:40 ` [PATCH 10/11] vfs: Remove d_drop calls from d_revalidate implementations Eric W. Biederman
2014-02-15 21:40 ` [PATCH 11/11] proc: Update proc_flush_task_mnt to use d_invalidate Eric W. Biederman
2014-02-18 17:43 ` [PATCH 0/11] Detaching mounts on unlink Miklos Szeredi
2014-02-25 9:33 ` [PATCH 0/12] Detaching mounts on unlink (for 3.15 v2) Eric W. Biederman
2014-02-25 9:34 ` [PATCH 01/12] vfs: Document the effect of d_revalidate on d_find_alias Eric W. Biederman
2014-02-25 9:35 ` [PATCH 02/12] vfs: More precise tests in d_invalidate Eric W. Biederman
2014-02-25 9:35 ` [PATCH 03/12] vfs: Don't allow overwriting mounts in the current mount namespace Eric W. Biederman
2014-02-25 9:36 ` [PATCH 04/12] vfs: Keep a list of mounts on a mount point Eric W. Biederman
2014-02-25 9:37 ` [PATCH 05/12] vfs: factor out lookup_mountpoint from new_mountpoint Eric W. Biederman
2014-02-25 9:38 ` [PATCH 06/12] vfs: Add a function to lazily unmount all mounts from any dentry Eric W. Biederman
2014-02-25 9:38 ` [PATCH 07/12] vfs: Lazily remove mounts on unlinked files and directories Eric W. Biederman
2014-02-25 9:39 ` [PATCH 08/12] vfs: Remove unnecessary calls of check_submounts_and_drop Eric W. Biederman
2014-02-25 9:40 ` [PATCH 09/12] vfs: Merge check_submounts_and_drop and d_invalidate Eric W. Biederman
2014-02-25 9:40 ` [PATCH 10/12] vfs: Make d_invalidate return void Eric W. Biederman
2014-02-25 9:41 ` [PATCH 11/12] vfs: Remove d_drop calls from d_revalidate implementations Eric W. Biederman
2014-02-25 9:41 ` [PATCH 12/12] proc: Update proc_flush_task_mnt to use d_invalidate Eric W. Biederman
2014-04-09 0:21 ` [GIT PULL] Detaching mounts on unlink for 3.15-rc1 Eric W. Biederman
2014-04-09 2:30 ` Al Viro
2014-04-09 2:39 ` Al Viro
2014-04-09 9:02 ` Eric W. Biederman
2014-04-09 17:32 ` Eric W. Biederman
2014-04-09 17:53 ` Al Viro
2014-04-09 18:28 ` Al Viro
2014-04-09 22:49 ` Eric W. Biederman
2014-04-09 22:58 ` [RFC][PATCH] vfs: In mntput run deactivate_super on a shallow stack Eric W. Biederman
2014-04-09 23:24 ` Al Viro
2014-04-10 1:36 ` Eric W. Biederman
2014-04-12 22:15 ` Eric W. Biederman
2014-04-13 5:39 ` Al Viro
2014-04-13 5:49 ` Al Viro
2014-04-13 7:01 ` Eric W. Biederman
2014-04-13 7:51 ` Eric W. Biederman
2014-04-13 21:52 ` Al Viro
2014-04-14 7:38 ` [RFC][PATCH 0/4] No I/O from mntput Eric W. Biederman
2014-04-14 7:40 ` [PATCH 1/4] vfs: Remove useless loop in mntput_no_expire Eric W. Biederman
2014-04-14 7:40 ` [PATCH 2/4] vfs: Move autoclose of BSD accounting into a work queue Eric W. Biederman
2014-04-14 7:41 ` [PATCH 3/4] vfs: In mntput run deactivate_super on a shallow stack Eric W. Biederman
2014-04-14 7:42 ` [PATCH 4/4] vfs: Block intuitively in the case of BSD accounting files Eric W. Biederman
2014-04-16 22:03 ` [RFC][PATCH 0/4] No I/O from mntput Eric W. Biederman
2014-04-17 20:05 ` [GIT PULL] Detaching mounts on unlink for 3.15 Eric W. Biederman
2014-04-17 20:22 ` Al Viro
2014-04-17 21:23 ` Eric W. Biederman
2014-04-17 22:12 ` Al Viro
2014-04-17 22:14 ` Al Viro
2014-04-18 0:37 ` Al Viro
2014-04-18 0:58 ` Al Viro
2014-04-19 1:35 ` Al Viro
2014-04-19 2:16 ` Al Viro
2014-04-19 7:05 ` Al Viro
2014-04-20 5:41 ` Al Viro
2014-05-11 16:45 ` Al Viro
2014-05-11 17:05 ` Al Viro
2014-08-09 9:34 ` Al Viro
2014-08-09 9:55 ` Eric W. Biederman
2014-08-12 10:17 ` Eric W. Biederman
2014-08-13 13:18 ` Al Viro
2014-08-13 17:18 ` Eric W. Biederman
2014-04-09 22:01 ` [GIT PULL] Detaching mounts on unlink for 3.15-rc1 Dave Chinner
2013-08-15 21:23 ` DoS with unprivileged mounts Rob Landley
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=87bo1u8vmf.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=containers@lists.linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=miklos@szeredi.hu \
--cc=ms@citd.de \
--cc=rob@landley.net \
--cc=serge@hallyn.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).