From patchwork Wed Aug 13 13:48:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Whitcroft X-Patchwork-Id: 379644 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id AA1A4140090; Wed, 13 Aug 2014 23:48:57 +1000 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1XHYv8-0005B2-In; Wed, 13 Aug 2014 13:48:54 +0000 Received: from mail-wg0-f45.google.com ([74.125.82.45]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1XHYv2-00058j-80 for kernel-team@lists.ubuntu.com; Wed, 13 Aug 2014 13:48:48 +0000 Received: by mail-wg0-f45.google.com with SMTP id x12so11163903wgg.28 for ; Wed, 13 Aug 2014 06:48:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=1ZeHqvWd2Zq9fNdlMIxXW8knB/gRUdrXRru6JE/m5zU=; b=Cr5mYVyWekaLxDRh6Ngjgzrfdppw4A6OrjKGTycTQP5pIXhGlGdAlG9mdIdRyJFGjY uK3gOqKk7/OEyNucz8VCefHlJGt7FZDRJ+35NGa2iKJY9UjJldqPvzmNWeSaTTdo9YVJ sxE1Yh0yt5JSihW76Tx9ZtUBfGYYlREINGVDP1WJpJou9riVOEc8TIICS0BVTKXtgL8M 5y0PoE2dk+wZJPh4cgFJ+iMGeygc/wBaYIWvfo46n3umxIRZZXjQcuB62se6eWBYiyNj XfWRKoNc9rLOFvYQsbAJAJ6pp0cBGQnLluefHVSGpWSkGAEu3ZQpG8WZk5NJGuOj5m+h jMSw== X-Gm-Message-State: ALoCoQlfpaduFVeF+GTh5FfG/YV+GVhM36YKYWPInyY9Xk4pIm7vB1rXNQx2JUVvS4DrYkn44wBy X-Received: by 10.180.39.172 with SMTP id q12mr5068897wik.55.1407937728093; Wed, 13 Aug 2014 06:48:48 -0700 (PDT) Received: from localhost ([2001:470:6973:2:6493:b95b:a90e:3bcb]) by mx.google.com with ESMTPSA id ub11sm68248382wib.1.2014.08.13.06.48.46 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 13 Aug 2014 06:48:47 -0700 (PDT) From: Andy Whitcroft To: kernel-team@lists.ubuntu.com Subject: [trusty 3/4] mnt: Correct permission checks in do_remount Date: Wed, 13 Aug 2014 14:48:37 +0100 Message-Id: <1407937718-9362-4-git-send-email-apw@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1407937718-9362-1-git-send-email-apw@canonical.com> References: <1407937718-9362-1-git-send-email-apw@canonical.com> Cc: Andy Whitcroft X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: "Eric W. Biederman" While invesgiating the issue where in "mount --bind -oremount,ro ..." would result in later "mount --bind -oremount,rw" succeeding even if the mount started off locked I realized that there are several additional mount flags that should be locked and are not. In particular MNT_NOSUID, MNT_NODEV, MNT_NOEXEC, and the atime flags in addition to MNT_READONLY should all be locked. These flags are all per superblock, can all be changed with MS_BIND, and should not be changable if set by a more privileged user. The following additions to the current logic are added in this patch. - nosuid may not be clearable by a less privileged user. - nodev may not be clearable by a less privielged user. - noexec may not be clearable by a less privileged user. - atime flags may not be changeable by a less privileged user. The logic with atime is that always setting atime on access is a global policy and backup software and auditing software could break if atime bits are not updated (when they are configured to be updated), and serious performance degradation could result (DOS attack) if atime updates happen when they have been explicitly disabled. Therefore an unprivileged user should not be able to mess with the atime bits set by a more privileged user. The additional restrictions are implemented with the addition of MNT_LOCK_NOSUID, MNT_LOCK_NODEV, MNT_LOCK_NOEXEC, and MNT_LOCK_ATIME mnt flags. Taken together these changes and the fixes for MNT_LOCK_READONLY should make it safe for an unprivileged user to create a user namespace and to call "mount --bind -o remount,... ..." without the danger of mount flags being changed maliciously. Cc: stable@vger.kernel.org Acked-by: Serge E. Hallyn Signed-off-by: "Eric W. Biederman" (cherry picked from commit 9566d6742852c527bf5af38af5cbb878dad75705) CVE-2014-5207 BugLink: http://bugs.launchpad.net/bugs/1356323 Signed-off-by: Andy Whitcroft --- fs/namespace.c | 36 +++++++++++++++++++++++++++++++++--- include/linux/mount.h | 5 +++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 36860ea..430301f 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -888,8 +888,21 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root, mnt->mnt.mnt_flags = old->mnt.mnt_flags & ~(MNT_WRITE_HOLD|MNT_MARKED); /* Don't allow unprivileged users to change mount flags */ - if ((flag & CL_UNPRIVILEGED) && (mnt->mnt.mnt_flags & MNT_READONLY)) - mnt->mnt.mnt_flags |= MNT_LOCK_READONLY; + if (flag & CL_UNPRIVILEGED) { + mnt->mnt.mnt_flags |= MNT_LOCK_ATIME; + + if (mnt->mnt.mnt_flags & MNT_READONLY) + mnt->mnt.mnt_flags |= MNT_LOCK_READONLY; + + if (mnt->mnt.mnt_flags & MNT_NODEV) + mnt->mnt.mnt_flags |= MNT_LOCK_NODEV; + + if (mnt->mnt.mnt_flags & MNT_NOSUID) + mnt->mnt.mnt_flags |= MNT_LOCK_NOSUID; + + if (mnt->mnt.mnt_flags & MNT_NOEXEC) + mnt->mnt.mnt_flags |= MNT_LOCK_NOEXEC; + } /* Don't allow unprivileged users to reveal what is under a mount */ if ((flag & CL_UNPRIVILEGED) && list_empty(&old->mnt_expire)) @@ -1951,6 +1964,23 @@ static int do_remount(struct path *path, int flags, int mnt_flags, !(mnt_flags & MNT_READONLY)) { return -EPERM; } + if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) && + !(mnt_flags & MNT_NODEV)) { + return -EPERM; + } + if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) && + !(mnt_flags & MNT_NOSUID)) { + return -EPERM; + } + if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) && + !(mnt_flags & MNT_NOEXEC)) { + return -EPERM; + } + if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) && + ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) { + return -EPERM; + } + err = security_sb_remount(sb, data); if (err) return err; @@ -2149,7 +2179,7 @@ static int do_new_mount(struct path *path, const char *fstype, int flags, */ if (!(type->fs_flags & FS_USERNS_DEV_MOUNT)) { flags |= MS_NODEV; - mnt_flags |= MNT_NODEV; + mnt_flags |= MNT_NODEV | MNT_LOCK_NODEV; } } diff --git a/include/linux/mount.h b/include/linux/mount.h index 5ea2a80..fff78cb 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -45,12 +45,17 @@ struct mnt_namespace; #define MNT_USER_SETTABLE_MASK (MNT_NOSUID | MNT_NODEV | MNT_NOEXEC \ | MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME \ | MNT_READONLY) +#define MNT_ATIME_MASK (MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME ) #define MNT_INTERNAL_FLAGS (MNT_SHARED | MNT_WRITE_HOLD | MNT_INTERNAL | \ MNT_DOOMED | MNT_SYNC_UMOUNT | MNT_MARKED) #define MNT_INTERNAL 0x4000 +#define MNT_LOCK_ATIME 0x040000 +#define MNT_LOCK_NOEXEC 0x080000 +#define MNT_LOCK_NOSUID 0x100000 +#define MNT_LOCK_NODEV 0x200000 #define MNT_LOCK_READONLY 0x400000 #define MNT_LOCKED 0x800000 #define MNT_DOOMED 0x1000000