Re: [PATCH v2 00/10] Fix dm-ima bugs

[Mikulas Patocka <mpatocka@redhat.com>]

Applied, thanks.

Mikulas



On Wed, 29 Apr 2026, Benjamin Marzinski wrote:

> The dm-ima code does not guarantee that the dm_ima_measure_on_*
> functions will not be called at the same time. Since they modify and
> free shared memory, this can lead to Use-After-Free errors or garbage
> measurements. Further, they don't make sure that the state they measure
> corresponds to the actual device state. For instance if table_load()
> runs at the same time as do_resume() on a table swap,
> dm_ima_measure_on_device_resume() can end up thinking the wrong table is
> active. Or a concurrent dm_hash_rename() and a table swap, can end up
> with a the new active table still using the old name. This patchset
> makes sure the the dm-ima function are serialized and report the correct
> device state.
> 
> However, the code is still messier that in could be. This is because
> it duplicates the current measurement events and format. I would really
> like to know if that is necessary. Specifically, it currently measures
> the following dm device and table actions:
> 
> load
> clear
> rename
> resume
> remove
> 
> I don't see the benefit of reporting changes to the inactive table, or
> resumes where the device does not change state. From the user's point of
> view, the device is still the same after these events.  At the same
> time, it doesn't measure device creates if no table was loaded, so you
> can have situations where the the first measurement for a device is a
> rename or a remove. A more sensible set of actions to measure would be:
> 
> create
> table_swap
> rename
> remove
> 
> Also, the measurement format doesn't map well to how dm device's are
> actually set up, in a way that makes it harder for the code and records
> extraneous information. First, like I mentioned before, I don't see the
> benefit of measuring the inactive table. Second, the name, uuid, and
> major/minor numbers are properties of the device, not it's table (and dm
> devices can't have partitions, so the minor count will always be 1). I
> don't see a reason to store and occasinally log this information twice,
> if there is an active and incative table, and it forces extra
> coordination between the dm_ima_measure_on_* functions.
> 
> I'm wondering it we are stuck with the current events and format, now
> that this has been released? Or could we bump the version, and change
> what events we measure, and how we format the output?
> 
> Changes in v2:
> - 05/10: Switched from using atomics and barriers to using a spinlock to
>          serialize the dm_ima_measure_on_* functions
> - 05/10: Fixed typos in comments and commit message
> - 06/10: changed no_data string from "table_rename" to "device_rename"
> 
> If this patchset is going in largely as it is, I should point out that
> there are changes to the ima measurements in corner cases, where the
> data was previously incorrect (as mentioned in the patches). However
> it's possible, although I don't think very likely, that systems are
> expecting these incorrect measurements. We could bump the version number
> because of this possible change in the measurements, but IIUC, since
> this version number itself is part of the IMA measurements, that would
> guarantee that systems would get different measurements. 
> 
> Benjamin Marzinski (10):
>   dm-ima: remove dm_ima_reset_data()
>   dm-ima: remove broken last_target_measured logic
>   dm-ima: Remove status_flags from dm_ima_measure_on_table_load()
>   dm-ima: don't copy the active table to the inactive table
>   dm-ima: Fix UAF errors and measuring incorrect context
>   dm-ima: remove new_map from dm_ima_measure_on_device_clear
>   dm-ima: Fix issues with dm_ima_measure_on_device_rename
>   dm-ima: Handle race between rename and table swap
>   dm-ima: Fail more gracefully in dm_ima_measure_on_*
>   dm-ima: use active table's size if available
> 
>  drivers/md/dm-ima.c   | 492 +++++++++++++++++++-----------------------
>  drivers/md/dm-ima.h   |  68 ++++--
>  drivers/md/dm-ioctl.c | 146 +++++++++++--
>  drivers/md/dm.c       |   2 +-
>  4 files changed, 410 insertions(+), 298 deletions(-)
> 
> -- 
> 2.53.0
>