public inbox for linux-integrity@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Fix dm-ima bugs
@ 2026-04-29 20:20 Benjamin Marzinski
  2026-04-29 20:20 ` [PATCH v2 01/10] dm-ima: remove dm_ima_reset_data() Benjamin Marzinski
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Benjamin Marzinski @ 2026-04-29 20:20 UTC (permalink / raw)
  To: Mikulas Patocka, Mike Snitzer
  Cc: dm-devel, linux-integrity, Alasdair Kergon, Mimi Zohar,
	Roberto Sassu, Dmitry Kasatkin, Lakshmi Ramasubramanian,
	steven chen

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


^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-04-29 20:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29 20:20 [PATCH v2 00/10] Fix dm-ima bugs Benjamin Marzinski
2026-04-29 20:20 ` [PATCH v2 01/10] dm-ima: remove dm_ima_reset_data() Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 02/10] dm-ima: remove broken last_target_measured logic Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 03/10] dm-ima: Remove status_flags from dm_ima_measure_on_table_load() Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 04/10] dm-ima: don't copy the active table to the inactive table Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 05/10] dm-ima: Fix UAF errors and measuring incorrect context Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 06/10] dm-ima: remove new_map from dm_ima_measure_on_device_clear Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 07/10] dm-ima: Fix issues with dm_ima_measure_on_device_rename Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 08/10] dm-ima: Handle race between rename and table swap Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 09/10] dm-ima: Fail more gracefully in dm_ima_measure_on_* Benjamin Marzinski
2026-04-29 20:21 ` [PATCH v2 10/10] dm-ima: use active table's size if available Benjamin Marzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox