On Tue, 2023-10-17 at 10:08 -0600, Raul E Rangel wrote:
> On Mon, Sep 11, 2023 at 03:38:20PM +0200, Greg Kroah-Hartman wrote:
> > 6.4-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
> >
> > [ Upstream commit 18b44bc5a67275641fb26f2c54ba7eef80ac5950 ]
> >
> > Commit db1d1e8b9867 ("IMA: use vfs_getattr_nosec to get the i_version")
> > partially closed an IMA integrity issue when directly modifying a file
> > on the lower filesystem. If the overlay file is first opened by a user
> > and later the lower backing file is modified by root, but the extended
> > attribute is NOT updated, the signature validation succeeds with the old
> > original signature.
> >
> > Update the super_block s_iflags to SB_I_IMA_UNVERIFIABLE_SIGNATURE to
> > force signature reevaluation on every file access until a fine grained
> > solution can be found.
> >
>
> Sorry for replying to the 6.4-stable patch, I couldn't find the original
> patch in the mailing list.
>
> We recently upgraded from 6.4.4 to 6.5.3. We have the integrity LSM
> enabled, and are using overlayfs. When we try and execute a binary from
> the overlayfs filesystem, the integrity LSM hashes the binary and all
> its shared objects every single invocation. This causes a serious
> performance regression when invoking clang thousands of times while
> building a package. We bisected the culprit down to this patch.
>
> Here are some numbers:
>
> With this patch + overlayfs:
>
> $ time /usr/bin/clang-17 --version > /dev/null
>
> real 0m0.628s
> user 0m0.004s
> sys 0m0.624s
> $ time /usr/bin/clang-17 --version > /dev/null
>
> real 0m0.597s
> user 0m0.004s
> sys 0m0.593s
>
> With this patch - overlayfs:
>
> $ truncate -s 1G foo.bin
> $ mkfs.ext4 foo.bin
> $ mount foo.bin /foo
> $ cp /usr/bin/clang-17 /foo
> $ time /foo/clang-17 --version > /dev/null
>
> real 0m0.040s
> user 0m0.009s
> sys 0m0.031s
> $ time /foo/clang-17 --version > /dev/null
>
> real 0m0.036s
> user 0m0.000s
> sys 0m0.037s
>
> Without this path + overlayfs:
> $ time /usr/bin/clang-17 --version > /dev/null
>
> real 0m0.017s
> user 0m0.007s
> sys 0m0.011s
> $ time /usr/bin/clang-17 --version > /dev/null
>
> real 0m0.018s
> user 0m0.000s
> sys 0m0.018s
>
> i.e., we go from ~30ms / invocation to 600ms / invocation. Building
> glibc used to take about 3 minutes, but now its taking about 20 minutes.
>
> Our clang binary is about 100 MiB in size.
>
> Using `perf` the following sticks out:
> $ perf record -g time /usr/bin/clang-17 --version
> --92.03%--elf_map
> vm_mmap_pgoff
> ima_file_mmap
> process_measurement
> ima_collect_measurement
> |
> --91.95%--ima_calc_file_hash
> ima_calc_file_hash_tfm
> |
> |--82.85%--_sha256_update
> | |
> | --82.47%--lib_sha256_base_do_update.isra.0
> | |
> | --82.39%--sha256_transform_rorx
> |
> --9.10%--integrity_kernel_read
>
> The audit.log is also logging every clang invocation as well.
>
> Was such a large performance regression expected? Can the commit be
> reverted until the more fine grained solution mentioned in the commit
> message be implemented?
IMA is always based on policy. Having the "integrity LSM enabled and
using overlayfs" will not cause any measurements or signature
verifications, unless the files are in policy.
The problem is that unless the lower layer file is in policy, file
change will not be detected on the overlay filesystem. Reverting this
change will allow access to a modified file without re-verifying its
integrity.
Instead of reverting the patch, perhaps allow users to take this risk
by defining a Kconfig, since they're aware of their policy rules.
--
thanks,
Mimi
