Re: IMA/EVM writing xattrs during remount filesystem

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

 



On Mon, Feb 26, 2018 at 10:12:05AM -0500, Mimi Zohar wrote:
> Hi Sascha,
> 
> On Mon, 2018-02-26 at 15:23 +0100, Sascha Hauer wrote:
> > Hi All,
> > 
> > When a filesystem is remounted from rw to ro then
> > sb_prepare_remount_readonly() is called. After this call there shouldn't
> > be any writers left on the filesystem. However, IMA/EVM is not aware of
> > this as it never calls mnt_want_write[_file](), but only looks add the
> > MS_RDONLY superblock flag before writing to its xattrs. This flag is
> > only changed after sb->s_op->remount_fs() is called. As a consequence
> > IMA/EVM still updates xattrs while the filesystem is going to readonly
> > mode.
> > 
> > We observed that on a 4.0 Kernel in conjunction with UBIFS, but the
> > relevant code in IMA/EVM still looks the same so I assume it's present
> > in the current kernel aswell.
> > 
> > UBIFS calculates its free space before and after the remount_fs op and
> > if there's a difference it prints a backtrace (dbg_check_space_info:
> > free space changed from x to y). We see this backtrace sometimes when
> > remounting the fs readonly. If I understand the situation correctly this
> > is not UBIFS's fault, right? Any hint what we can do about it?
> 
> Not updating the file hashes could result in verification errors.  I
> would classify updating the xattrs as working as designed.  Wouldn't
> you?

If IMA updates the hash for a file that actually gets written then the
file is opened for writing. Trying remount the filesystem readonly in
this situation will return -EBUSY. Everything fine here, but look at
evm_verify_hmac(). Here a file is not necessarily opened for writing,
but if the file is signed the code replaces the signature with a HMAC by
calling evm_update_evmxattr(). This function updates the xattr even when
the VFS is in the process of remounting the filesystem readonly.

More specifically look at do_remount_sb():

>	/* If we are remounting RDONLY and current sb is read/write,
>	   make sure there are no rw files opened */
>	if (remount_ro) {
>		if (force) {
>			sb->s_readonly_remount = 1;
>			smp_wmb();
>		} else {
>			retval = sb_prepare_remount_readonly(sb);
>			if (retval)
>				return retval;
>		}
>	}

When sb_prepare_remount_readonly() succeeds there are no writers left.

>
>	if (sb->s_op->remount_fs) {
>		retval = sb->s_op->remount_fs(sb, &sb_flags, data);
>		if (retval) {
>			if (!force)
>				goto cancel_readonly;
>			/* If forced remount, go ahead despite any errors */
>			WARN(1, "forced remount of a %s fs returned %i\n",
>			     sb->s_type->name, retval);
>		}
>	}

remount_fs assumes VFS has stopped writing. At least UBIFS expects this,
maybe it's wrong:

/**
 * ubifs_remount_ro - re-mount in read-only mode.
 * @c: UBIFS file-system description object
 *
 * We assume VFS has stopped writing. Possibly the background thread could be
 * running a commit, however kthread_stop will wait in that case.
 */

>	sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (sb_flags & MS_RMT_MASK);

Here, *after* remount_fs has returned the MS_RDONLY sb flag is set which
EVM tests for before calling evm_update_evmxattr() and the race window
closes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux