Skip to content

Commit 9566d67

Browse files
committed
mnt: Correct permission checks in do_remount
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 <serge.hallyn@ubuntu.com> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
1 parent 07b6455 commit 9566d67

File tree

2 files changed

+38
-3
lines changed

2 files changed

+38
-3
lines changed

Diff for: fs/namespace.c

+33-3
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,21 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
890890

891891
mnt->mnt.mnt_flags = old->mnt.mnt_flags & ~(MNT_WRITE_HOLD|MNT_MARKED);
892892
/* Don't allow unprivileged users to change mount flags */
893-
if ((flag & CL_UNPRIVILEGED) && (mnt->mnt.mnt_flags & MNT_READONLY))
894-
mnt->mnt.mnt_flags |= MNT_LOCK_READONLY;
893+
if (flag & CL_UNPRIVILEGED) {
894+
mnt->mnt.mnt_flags |= MNT_LOCK_ATIME;
895+
896+
if (mnt->mnt.mnt_flags & MNT_READONLY)
897+
mnt->mnt.mnt_flags |= MNT_LOCK_READONLY;
898+
899+
if (mnt->mnt.mnt_flags & MNT_NODEV)
900+
mnt->mnt.mnt_flags |= MNT_LOCK_NODEV;
901+
902+
if (mnt->mnt.mnt_flags & MNT_NOSUID)
903+
mnt->mnt.mnt_flags |= MNT_LOCK_NOSUID;
904+
905+
if (mnt->mnt.mnt_flags & MNT_NOEXEC)
906+
mnt->mnt.mnt_flags |= MNT_LOCK_NOEXEC;
907+
}
895908

896909
/* Don't allow unprivileged users to reveal what is under a mount */
897910
if ((flag & CL_UNPRIVILEGED) && list_empty(&old->mnt_expire))
@@ -1931,6 +1944,23 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
19311944
!(mnt_flags & MNT_READONLY)) {
19321945
return -EPERM;
19331946
}
1947+
if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) &&
1948+
!(mnt_flags & MNT_NODEV)) {
1949+
return -EPERM;
1950+
}
1951+
if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) &&
1952+
!(mnt_flags & MNT_NOSUID)) {
1953+
return -EPERM;
1954+
}
1955+
if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) &&
1956+
!(mnt_flags & MNT_NOEXEC)) {
1957+
return -EPERM;
1958+
}
1959+
if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) &&
1960+
((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) {
1961+
return -EPERM;
1962+
}
1963+
19341964
err = security_sb_remount(sb, data);
19351965
if (err)
19361966
return err;
@@ -2129,7 +2159,7 @@ static int do_new_mount(struct path *path, const char *fstype, int flags,
21292159
*/
21302160
if (!(type->fs_flags & FS_USERNS_DEV_MOUNT)) {
21312161
flags |= MS_NODEV;
2132-
mnt_flags |= MNT_NODEV;
2162+
mnt_flags |= MNT_NODEV | MNT_LOCK_NODEV;
21332163
}
21342164
}
21352165

Diff for: include/linux/mount.h

+5
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,17 @@ struct mnt_namespace;
4545
#define MNT_USER_SETTABLE_MASK (MNT_NOSUID | MNT_NODEV | MNT_NOEXEC \
4646
| MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME \
4747
| MNT_READONLY)
48+
#define MNT_ATIME_MASK (MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME )
4849

4950
#define MNT_INTERNAL_FLAGS (MNT_SHARED | MNT_WRITE_HOLD | MNT_INTERNAL | \
5051
MNT_DOOMED | MNT_SYNC_UMOUNT | MNT_MARKED)
5152

5253
#define MNT_INTERNAL 0x4000
5354

55+
#define MNT_LOCK_ATIME 0x040000
56+
#define MNT_LOCK_NOEXEC 0x080000
57+
#define MNT_LOCK_NOSUID 0x100000
58+
#define MNT_LOCK_NODEV 0x200000
5459
#define MNT_LOCK_READONLY 0x400000
5560
#define MNT_LOCKED 0x800000
5661
#define MNT_DOOMED 0x1000000

0 commit comments

Comments
 (0)