Adding Eric to cc.
On 7/7/23 06:00, Mimi Zohar wrote:
Hi Tushar,
On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
There is no existing IMA functionality to just populate the buffer at
kexec execute with IMA measurements.
The same function that copies the measurement list at kexec 'load',
could be re-used at kexec 'exec'. Why is a new function that is very
similar to the existing ima_dump_measurement_list() needed?
The current implementation of ima_dump_measurement_list() does both -
(1) allocation of buffer and (2) copying the measurements at kexec ‘load’.
The goal is to copy the measurements at kexec ‘execute’.
As I mentioned earlier, my understanding is the buffer must be allocated
at kexec ‘load’ time. And the segment size cannot change between kexec
‘load’ and kexec ‘execute’.
That’s why I believe the two new functions are needed.
So I split the functionality in ima_dump_measurement_list() into
two functions –
(1) ima_allocate_buf_at_kexec_load() in patch 01/10 and
(2) ima_populate_buf_at_kexec_execute() in patch 02/10
and then deprecated ima_dump_measurement_list() in patch 7/10.
If I call ima_dump_measurement_list() both at kexec ‘load’
as well as kexec ‘execute’, it will result in double memory allocation.
As you said, if the functionality can be achieved by simply moving
ima_dump_measurement_list() from kexec ‘load’ to kexec ‘execute’ –
then you are right, we don’t need the two separate functions.
~Tushar
Implement a function to iterate over ima_measurements and populate the
ima_kexec_file buffer. After the loop, populate ima_khdr with buffer
details (version, buffer size, number of measurements). Copy the ima_khdr
data into ima_kexec_file.buf and update buffer_size and buffer.
The patch assumes that the ima_kexec_file.size is sufficient to hold all
the measurements. It returns an error and does not handle scenarios where
additional space might be needed.
Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>
---
security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 48a683874044..858b67689701 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
return 0;
}
+static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
+{
+ struct ima_queue_entry *qe;
+ int ret = 0;
+
+ /*
+ * Ensure the kexec buffer is large enough to hold ima_khdr
+ */
+ if (ima_kexec_file.size < sizeof(ima_khdr)) {
+ pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
+ __func__);
+ ima_clear_kexec_file();
+ return -ENOMEM;
+ }
+
+ list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ if (ima_kexec_file.count < ima_kexec_file.size) {
+ ima_khdr.count++;
+ ima_measurements_show(&ima_kexec_file, qe);
+ } else {
+ ret = -ENOMEM;
+ pr_err("%s: Kexec ima_measurements buffer too small\n",
+ __func__);
+ break;
+ }
+ }
+ if (ret < 0)
+ goto out;
+
+ /*
+ * fill in reserved space with some buffer details
+ * (eg. version, buffer size, number of measurements)
+ */
+ ima_khdr.buffer_size = ima_kexec_file.count;
+ if (ima_canonical_fmt) {
+ ima_khdr.version = cpu_to_le16(ima_khdr.version);
+ ima_khdr.count = cpu_to_le64(ima_khdr.count);
+ ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
+ }
+
+ memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
+ *buffer_size = ima_kexec_file.count;
+ *buffer = ima_kexec_file.buf;
+
+out:
+ if (ret < 0)
+ ima_clear_kexec_file();
+
+ return ret;
+}
+
b+
static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
unsigned long segment_size)
{
