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 |
