Re: [PATCH] mnt: Prevent pivot_root from creating a loop in the mount tree

[ebiederm@xmission.com (Eric W. Biederman)]

Andy Lutomirski <luto@amacapital.net> writes:

> On Wed, Oct 8, 2014 at 10:42 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>>
>> Andy Lutomirski recently demonstrated that when chroot is used to set
>> the root path below the path for the new ``root'' passed to pivot_root
>> the pivot_root system call succeeds and leaks mounts.
>
> As part of my security fix what-happened-to-it quest: what happened to
> this fix?

On my part I am in the middle of a move right now and I don't have time
to push it to Linus.

But I will mention quickly that the fix below addresses your question of
how should pivot_root behave if chrooted because I continue to allow the
cases that actually work. (Not that I think anyone cares but this is
what we have).

> --Andy
>
>>
>> In examining the code I see that starting with a new root that is
>> below the current root in the mount tree will result in a loop in the
>> mount tree after the mounts are detached and then reattached to one
>> another.  Resulting in all kinds of ugliness including a leak of that
>> mounts involved in the leak of the mount loop.
>>
>> Prevent this problem by ensuring that the new mount is reachable from
>> the current root of the mount tree.
>>
>> Reported-by: Andy Lutomirski <luto@amacapital.net>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> ---
>>  fs/namespace.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index b3bdda8b5a01..7b776285832e 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -2830,6 +2830,9 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
>>         /* make sure we can reach put_old from new_root */
>>         if (!is_path_reachable(old_mnt, old.dentry, &new))
>>                 goto out4;
>> +       /* make certain new is below the root */
>> +       if (!is_path_reachable(new_mnt, new.dentry, &root))
>> +               goto out4;
>>         root_mp->m_count++; /* pin it so it won't go away */
>>         lock_mount_hash();
>>         detach_mnt(new_mnt, &parent_path);
>> --
>> 1.9.1
>>