ima/evm: security.ima and security.evm get written to frozen filesystems

[Christian Brauner]

Hey,

I'm working on some freeze protection fixes that plague the tree.
IMA and EVM update security.ima and security.evm without freeze
protection. IOW, they modify filesystems that are frozen. On ext4 that
trips the WARN_ON() in ext4_journal_check_start().

The update on the last close:

__fput()
-> security_file_release()
   -> ima_file_free()
      -> ima_check_last_writer()                  # iint->mutex
         -> ima_update_xattr()                    # inode lock
            -> ima_fix_xattr()
               -> __vfs_setxattr_noperm(security.ima)
                  -> evm_inode_post_setxattr()
                     -> evm_update_evmxattr()

Nothing on that path holds sb_writers. Write to a file covered by an
appraise policy, freeze the filesystem, close the file.

The same happens when files are appraised:

- fix mode: ima_appraise_measurement() -> ima_fix_xattr() and
  evm_fix_hmac()
- evm_verify_hmac() converting a digsig into an HMAC

for open, exec, mmap and kernel_read_file(). Only an O_TRUNC open of an
existing file holds sb_writers there.

Reproducer on v7.4-rc1, booted with ima_appraise=fix, ext4 on /dev/vdb:

    mount /dev/vdb /mnt
    echo "appraise fsuuid=$(blkid -s UUID -o value /dev/vdb)" > /sys/kernel/security/ima/policy
    echo a > /mnt/f; sync
    exec 3>>/mnt/f; echo b >&3
    fsfreeze -f /mnt
    exec 3>&-

The close rewrites security.ima on the frozen filesystem:

  WARNING: fs/ext4/ext4_jbd2.c:90 at ext4_journal_check_start+0x1f6/0x2d0, CPU#0: bash/280
  Call Trace:
   __ext4_journal_start_sb+0x7a/0x470
   ext4_xattr_set+0x20d/0x380
   __vfs_setxattr+0x10f/0x170
   __vfs_setxattr_noperm+0xf4/0x5e0
   ima_update_xattr+0x1bb/0x2a0
   ima_file_free+0x2a7/0x3c0
   security_file_release+0x55/0x60
   __fput+0x2d6/0xab0
   fput_close_sync+0xf2/0x1c0
   filp_close_sync+0x5c/0x90
   __x64_sys_close+0x71/0xe0
   do_syscall_64+0x10e/0x5e0

Removing security.ima from a file and opening it while frozen does the
same from the appraisal:

   __vfs_setxattr_noperm+0xf4/0x5e0
   ima_appraise_measurement+0x2d2/0x36a0
   process_measurement+0x15b8/0x2120
   ima_file_check+0xbc/0x110
   security_file_post_open+0x3f/0x90
   path_openat+0x4ab/0x2300

I don't think this has a trivial fix. iint->mutex nests inside
sb_writers: do_open() holds mnt_want_write() for O_TRUNC across
security_file_post_open() -> ima_file_check() -> process_measurement().

So taking sb_start_write() under iint->mutex or under the inode lock
inverts the order and deadlocks against a pending freeze:

- close holds iint->mutex and waits in sb_start_write()
- an O_TRUNC open holds sb_writers and waits for iint->mutex
- the freezer waits for the open

Skipping the update while frozen isn't great either.
ima_check_last_writer() has already cleared IMA_UPDATE_XATTR by then and
nothing redoes the update, so the next open in enforce mode fails with
-EACCES on the stale hash.

For close the only thing I see is taking sb_start_write() before
iint->mutex when an update may be needed. That makes close() block
while the filesystem is frozen, which xfs_file_release() goes out of
its way to avoid.

The appraisal-time fixups can use sb_start_write_trylock() and skip when
frozen since they're best effort anyway.

This goes back to 2fe5d6def167 ("ima: integrity appraisal extension").