Linux-Security-Module Archive on
 help / color / Atom feed
From: (Eric W. Biederman)
To: Al Viro <>
Cc: Tetsuo Handa <>,
	David Howells <>,,,,
Subject: Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
Date: Mon, 08 Jul 2019 12:12:21 -0500
Message-ID: <> (raw)
In-Reply-To: <> (Al Viro's message of "Mon, 8 Jul 2019 14:18:31 +0100")

Al Viro <> writes:

> On Mon, Jul 08, 2019 at 09:02:10PM +0900, Tetsuo Handa wrote:
>> Hello, David Howells.
>> I realized via that a new set of
>> system calls for filesystem mounting has been added to Linux 5.2. But
>> I feel that LSM modules are not ready to support these system calls.
>> An example is move_mount() added by this patch. This patch added
>> security_move_mount() LSM hook but none of in-tree LSM modules are
>> providing "LSM_HOOK_INIT(move_mount, ...)" entry. Therefore, currently
>> security_move_mount() is a no-op. At least for TOMOYO, I want to check
>> mount manipulations caused by system calls because allowing mounts on
>> arbitrary location is not acceptable for pathname based access control.
>> What happened? I want TOMOYO to perform similar checks like mount() does.
> That would be like tomoyo_check_mount_acl(), right?
>         if (need_dev) {
>                 /* Get mount point or device file. */
>                 if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
>                         error = -ENOENT;
>                         goto out;
>                 }
>                 obj.path1 = path;
>                 requested_dev_name = tomoyo_realpath_from_path(&path);
>                 if (!requested_dev_name) {
>                         error = -ENOENT;
>                         goto out;
>                 }
>         } else {
> is an obvious crap for *ALL* cases.  You are doing pathname resolution,
> followed by normalization and checks.  Then the result of said pathname
> resolution is thrown out and it's redone (usually by something in fs/super.c).
> Results of _that_ get used.
> Could you spell TOCTOU?  And yes, exploiting that takes a lot less than
> being able to do mount(2) in the first place - just pass it
> /proc/self/fd/69/<some acceptable path>/. with descriptor refering to
> opened root directory.  With ~/<some acceptable path> being a symlink
> to whatever you actually want to hit.  And descriptor 42 being your
> opened homedir.  Now have that call of mount(2) overlap with dup2(42, 69)
> from another thread sharing your descriptor table.  It doesn't take much
> to get the timing right, especially if you can arrange for some other
> activity frequently hitting namespace_sem at least shared (e.g. reading
> /proc/mounts in a loop from another process); that's likely to stall
> mount(2) at the point of lock_mount(), which comes *AFTER* the point
> where LSM hook is stuck into.
> Again, *ANY* checks on "dev_name" in ->sb_mount() instances are so much
> snake oil.  Always had been.

Al you do realize that the TOCTOU you are talking about comes the system
call API.  TOMOYO can only be faulted for not playing in their own
sandbox and not reaching out and fixing the vfs implementation details.
Userspace has always had to very careful to only mount filesystems
on paths that root completely controls and won't change.

Further system calls for manipulating the mount tree have historically
done a crap job of validating their inputs.  Relying on the fact that
only root can call them.  So the idea of guarding against root doing
something silly was silly.

So I figure at the end of the day if the new security hooks for the new
mount system calls don't make it possible to remove the TOCTOU that is
on you and Dave.  You two touched that code last after all.

Not updating the new security hooks to at least do as good a job as the
old security hooks is the definition of regression.

So Al.  Please simmer down and take the valid criticism.


  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19 17:08 [PATCH 00/10] VFS: Provide new mount UAPI David Howells
2019-02-19 17:08 ` [PATCH 01/10] vfs: syscall: Add open_tree(2) to reference or clone a mount David Howells
2019-02-19 17:08 ` [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around David Howells
2019-02-20 12:32   ` Alan Jenkins
2019-02-20 12:41     ` Alan Jenkins
2019-02-20 16:23   ` Jann Horn
2019-07-08 12:02   ` Tetsuo Handa
2019-07-08 13:18     ` Al Viro
2019-07-08 17:12       ` Eric W. Biederman [this message]
2019-07-08 18:01         ` Al Viro
2019-07-08 18:13           ` Al Viro
2019-07-08 20:21           ` Al Viro
2019-07-09  0:13             ` Eric W. Biederman
2019-07-09 10:51               ` Tetsuo Handa
2019-07-22 10:12                 ` Tetsuo Handa
2019-07-23  4:16                   ` John Johansen
2019-07-23 13:45                     ` Tetsuo Handa
2019-08-06 10:43                       ` Tetsuo Handa
2019-08-09 15:44                         ` [PATCH] LSM: Disable move_mount() syscall when TOMOYO or AppArmor is enabled Tetsuo Handa
2019-08-22  3:51                         ` [RFC][PATCH] fix d_absolute_path() interplay with fsmount() Al Viro
2019-08-30 10:11                           ` Tetsuo Handa
2019-07-23 21:45             ` [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around James Morris
2019-07-23 23:30               ` Al Viro
2019-02-19 17:08 ` [PATCH 03/10] teach move_mount(2) to work with OPEN_TREE_CLONE David Howells
2019-02-20 18:59   ` Alan Jenkins
2019-02-26 17:45   ` Alan Jenkins
2019-02-19 17:08 ` [PATCH 04/10] Make anon_inodes unconditional David Howells
2019-02-19 17:09 ` [PATCH 05/10] vfs: syscall: Add fsopen() to prepare for superblock creation David Howells
2019-02-19 17:09 ` [PATCH 06/10] vfs: Implement logging through fs_context David Howells
2019-02-19 17:09 ` [PATCH 07/10] vfs: syscall: Add fsconfig() for configuring and managing a context David Howells
2019-02-19 17:09 ` [PATCH 08/10] vfs: syscall: Add fsmount() to create a mount for a superblock David Howells
2019-02-19 17:09 ` [PATCH 09/10] vfs: syscall: Add fspick() to select a superblock for reconfiguration David Howells
2019-02-19 17:09 ` [PATCH 10/10] vfs: Add a sample program for the new mount API David Howells

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on

Archives are clonable:
	git clone --mirror linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ \
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone