Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: > On Tue, Oct 14, 2014 at 12:57 PM, Eric W. Biederman > <ebiederm@xxxxxxxxxxxx> wrote: >> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: >> >>> On Wed, Oct 8, 2014 at 10:42 AM, Eric W. Biederman >>> <ebiederm@xxxxxxxxxxxx> 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). > > FWIW, the patch is: > > Reviewed-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx> > > Linus, Al, can one of you take this? Eric, are you okay with that? I have no objections. The patch is tested and works from my side, I just don't have time to retransmit right now. > Also, this needs Cc: stable@xxxxxxxxxxxxxxx, I believe. Agreed. > (No point in > limiting it to 3.8 and newer -- this is a bug bug, not just a security > bug, and based on my extremely limited understanding of how Docker and > lxc work, this could be exploitable even without user namespaces. Of > course, containers set up like that are giant security problems > regardless.) Andy you might want to add your Reviewed-by and the Cc: stable and stuff this patch in a git tree so Linus or whomever can grab it. My regrets for not having more time to push this. Eric >>> --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@xxxxxxxxxxxxxx> >>>> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> >>>> --- >>>> 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 >>>> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html