Re: [PATCH 3.8 10/13] mnt: Correct permission checks in do_remount

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

 



Hello Zefan Li,

This patch migh me needed for 3.4 too.

Thanks

On 09/03/2014 06:30 PM, Francis Moreau wrote:
> Hello,
> 
> Is it needed by 3.2.x, 3.4.x, 3.14.x ?
> 
> Thanks
> 
> On 08/25/2014 06:54 PM, Kamal Mostafa wrote:
>> 3.8.13.28 -stable review patch.  If anyone has any objections, please let me know.
>>
>> ------------------
>>
>> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>>
>> commit 9566d6742852c527bf5af38af5cbb878dad75705 upstream.
>>
>> 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.
>>
>> Acked-by: Serge E. Hallyn <serge.hallyn@xxxxxxxxxx>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>> Signed-off-by: Kamal Mostafa <kamal@xxxxxxxxxxxxx>
>> ---
>>  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 9171ac3..1d8b3d8 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -799,8 +799,21 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
>>  
>>  	mnt->mnt.mnt_flags = old->mnt.mnt_flags & ~MNT_WRITE_HOLD;
>>  	/* 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;
>> +	}
>>  
>>  	atomic_inc(&sb->s_active);
>>  	mnt->mnt.mnt_sb = sb;
>> @@ -1778,6 +1791,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;
>> @@ -1978,7 +2008,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 16fc05d..f058e13 100644
>> --- a/include/linux/mount.h
>> +++ b/include/linux/mount.h
>> @@ -45,10 +45,15 @@ 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	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
>>  
>>  struct vfsmount {
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]