Re: [PATCH v5 6/8] ima: suspend measurements during buffer copy at kexec execute

From: "Petr Tesařík" <petr@tesarici.cz>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: Tushar Sugandhi <tusharsu@linux.microsoft.com>,
	roberto.sassu@huaweicloud.com, roberto.sassu@huawei.com,
	eric.snowberg@oracle.com, stefanb@linux.ibm.com,
	ebiederm@xmission.com, noodles@fb.com, bauermann@kolabnow.com,
	linux-integrity@vger.kernel.org, kexec@lists.infradead.org,
	code@tyhicks.com, nramas@linux.microsoft.com,
	paul@paul-moore.com
Subject: Re: [PATCH v5 6/8] ima: suspend measurements during buffer copy at kexec execute
Date: Thu, 29 Feb 2024 14:21:16 +0100	[thread overview]
Message-ID: <20240229142116.5f16061b@meshulam.tesarici.cz> (raw)
In-Reply-To: <20b13e7bab048b3afb94a8878299aa7d61016364.camel@linux.ibm.com>

On Thu, 22 Feb 2024 11:38:23 -0500
Mimi Zohar <zohar@linux.ibm.com> wrote:

> > > @@ -176,6 +195,19 @@ int ima_add_template_entry(struct ima_template_entry
> > > *entry, int violation,
> > >  		}
> > >  	}
> > >  
> > > +	/*
> > > +	 * suspend_ima_measurements will be set if the system is
> > > +	 * undergoing kexec soft boot to a new kernel.
> > > +	 * suspending measurements in this short window ensures the
> > > +	 * consistency of the IMA measurement list during copying
> > > +	 * of the kexec buffer.
> > > +	 */  
> > 
> > Either remove the 2nd sentence "suspending measurements in this short window
> > ..." or explain what is meant by "short window".
> > 
> >   
> > > +	if (atomic_read(&suspend_ima_measurements)) {
> > > +		audit_cause = "measurements_suspended";
> > > +		audit_info = 0;
> > > +		goto out;  
> 
> After the suggested changes, understanding how many measurements are not being
> added to the measurement list and not being extended into the TPM would be
> really interesting.

First, I'm sorry for chiming in when v5 is already around, but I have
just found this patch series now.

It indeed sounds conceptually wrong to suspend and resume measurements.
At some point during the handover, other CPUs are taken offline (look
for migrate_to_reboot_cpu() in kernel/kexec_core.c) and even the reboot
CPU will be sufficiently shut down as not to be able to add any more
measurements.

IMO it would make more sense to copy the measurement list at this late
stage, even if it means adding a new notifier list (or a new action).

It may be a bit challenging if you want to make 100% sure that a new
measurement cannot be made from hard interrupt context, but is that even
a supported scenario?

Just my two (euro)cents,
Petr T