Hi Mike, On 7/20/21 2:27 PM, Mike Snitzer wrote: > On Mon, Jul 12 2021 at 8:48P -0400, > Tushar Sugandhi wrote: > >> For a given system, various external services/infrastructure tools >> (including the attestation service) interact with it - both during the >> setup and during rest of the system run-time. They share sensitive data >> and/or execute critical workload on that system. The external services >> may want to verify the current run-time state of the relevant kernel >> subsystems before fully trusting the system with business-critical >> data/workload. >> >> Device mapper is one such kernel subsystem that plays a critical role on >> a given system by providing various important functionalities to the >> block devices with various target types like crypt, verity, integrity >> etc. Each of these target types’ functionalities can be configured with >> various attributes. The attributes chosen to configure these target types >> can significantly impact the security profile of the block device, >> and in-turn, of the system itself. For instance, the type of encryption >> algorithm and the key size determines the strength of encryption for a >> given block device. >> >> Therefore, verifying the current state of various block devices as well >> as their various target attributes is crucial for external services >> before fully trusting the system with business-critical data/workload. >> >> IMA provides the necessary functionality for device mapper to measure the >> state and configuration of various block devices - >> - BY device mapper itself, from within the kernel, >> - in a tamper resistant way, >> - and re-measured - triggered on state/configuration change. >> >> This patch series uses this IMA functionality, by calling the function >> ima_measure_critical_data(), when a block device state is changed (e.g. >> on device create, resume, rename, remove etc.) It measures the device >> state and configuration and stores it in IMA logs, so that it can be >> used by external services for managing the system. > I needed to EXPORT_SYMBOL_GPL(ima_measure_critical_data); otherwise I > couldn't compile.. not sure but I can only imagine you compile DM > (and all targets) to be builtin? I believe the EXPORT_SYMBOL_GPL(ima_measure_critical_data) is now needed because of the move “#ifdef CONFIG_IMA from dm-ima.c to dm-ima.h”, and the associated change in the Makefile +ifeq ($(CONFIG_IMA),y) dm-mod-objs += dm-ima.o +endif Earlier I needed to export symbol only if I was calling ima_measure_critical_data() directly from various dm-*.c files. With my earlier implementation, it wasn’t needed when it was called from the wrapper dm_ima_measure_data() in dm-ima.c. > In addition to fixing that (in first table load patch) I changed > various things along the way while I reviewed each patch. Thank you so much for making the necessary changes Mike. Really appreciate it. > Things that I recall are: > - moved #ifdef CONFIG_IMA from dm-ima.c to dm-ima.h > - fixed various typos and whitespace > - consistently prepend "," in STATUSTYPE_IMA's DMEMIT()s as opposed to > having a mix or pre and postfix throughout targets > - fixed what seemed like malformed STATUSTYPE_IMA handling for > dm-multipath -- it was DMEMIT(";") for each dm-mpath's pathgroup > - added some fields to dm-mpath, renamed some IMA names in name=value > pairs to be more clear. I went through the changes you made. They look fine. In addition to the above changes, you also fixed a warning in dm-raid.c. (initializing the recovery variable) Thanks again. :) > I've staged the result in linux-next via linux-dm.git's dm-5.15 > branch, see: > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.15 > > I've compiled tested both with and without CONFIG_IMA set. But > haven't actually tested the code. No worries. I will test the changesthoroughly again. > Please send any incremental fixes relative to the dm-5.15 branch and > I'll get them folded in where appropriate. Ok. I will send incremental patches against dm-5.15 as you've suggested. Thanks again for the review and all the help so far. Really appreciate it. ~Tushar > Thanks, > Mike