Re: [REVIEW][PATCH 1/5] mnt: Only change user settable mount flags in remount

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Serge Hallyn <serge.hallyn@xxxxxxxxxx> writes:

> Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
>> 
>> Kenton Varda <kenton@xxxxxxxxxxxx> discovered that by remounting a
>> read-only bind mount read-only in a user namespace the
>> MNT_LOCK_READONLY bit would be cleared, allowing an unprivileged user
>> to the remount a read-only mount read-write.
>> 
>> Correct this by replacing the mask of mount flags to preserve
>> with a mask of mount flags that may be changed, and preserve
>> all others.   This ensures that any future bugs with this mask and
>> remount will fail in an easy to detect way where new mount flags
>> simply won't change.
>> 
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>
> Not exactly sure about the name.  Actually seems like it should be
> caled MNT_USER_UNCLEARABLE_MASK or something, but

It is the set of mnt_flags that user space can cause to change.
So this patch inverts that mask and unconditionally preserves
everything else.

> Acked-by: Serge E. Hallyn <serge.hallyn@xxxxxxxxxx>
>
>> ---
>>  fs/namespace.c        | 2 +-
>>  include/linux/mount.h | 4 +++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 7187d01329c3..cb40449ea0df 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1937,7 +1937,7 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
>>  		err = do_remount_sb(sb, flags, data, 0);
>>  	if (!err) {
>>  		lock_mount_hash();
>> -		mnt_flags |= mnt->mnt.mnt_flags & MNT_PROPAGATION_MASK;
>> +		mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK;
>>  		mnt->mnt.mnt_flags = mnt_flags;
>>  		touch_mnt_namespace(mnt->mnt_ns);
>>  		unlock_mount_hash();
>> diff --git a/include/linux/mount.h b/include/linux/mount.h
>> index 839bac270904..b637a89e1fae 100644
>> --- a/include/linux/mount.h
>> +++ b/include/linux/mount.h
>> @@ -42,7 +42,9 @@ struct mnt_namespace;
>>   * flag, consider how it interacts with shared mounts.
>>   */
>>  #define MNT_SHARED_MASK	(MNT_UNBINDABLE)
>> -#define MNT_PROPAGATION_MASK	(MNT_SHARED | MNT_UNBINDABLE)
>> +#define MNT_USER_SETTABLE_MASK  (MNT_NOSUID | MNT_NODEV | MNT_NOEXEC \
>> +				 | MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME \
>> +				 | MNT_READONLY)
>>  
>>  #define MNT_INTERNAL_FLAGS (MNT_SHARED | MNT_WRITE_HOLD | MNT_INTERNAL | \
>>  			    MNT_DOOMED | MNT_SYNC_UMOUNT | MNT_MARKED)
>> -- 
>> 1.9.1
>> 
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers




[Index of Archives]     [Cgroups]     [Netdev]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux