diff mbox series

[07/10] ima: remove function ima_dump_measurement_list

Message ID 20230703215709.1195644-8-tusharsu@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series ima: measure events between kexec load and execute | expand

Commit Message

Tushar Sugandhi July 3, 2023, 9:57 p.m. UTC
The ima_dump_measurement_list function was designed to iterate over the
IMA measurement list and store each entry into a buffer.  The buffer,
along with its size, would be returned to the caller.  However, the
function is no longer required in the IMA subsystem.  It previously served
to dump the measurement list during the kexec 'load' operation, but this
functionality has been replaced by an alternative approach in this patch
series.

Remove the unused ima_dump_measurement_list function from the IMA
subsystem, to ensure a cleaner and more maintainable code.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 58 ------------------------------
 1 file changed, 58 deletions(-)

Comments

Mimi Zohar July 7, 2023, 1:55 p.m. UTC | #1
Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> The ima_dump_measurement_list function was designed to iterate over the
> IMA measurement list and store each entry into a buffer.  The buffer,
> along with its size, would be returned to the caller.  However, the
> function is no longer required in the IMA subsystem.  It previously served
> to dump the measurement list during the kexec 'load' operation, but this
> functionality has been replaced by an alternative approach in this patch
> series.
> 
> Remove the unused ima_dump_measurement_list function from the IMA
> subsystem, to ensure a cleaner and more maintainable code.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Have you compiled the code and tested after applying each patch? 
Removing the caller before removing the code should have resulted in a
warning.
Tushar Sugandhi July 11, 2023, 7:11 p.m. UTC | #2
Adding Eric to cc.


On 7/7/23 06:55, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> The ima_dump_measurement_list function was designed to iterate over the
>> IMA measurement list and store each entry into a buffer.  The buffer,
>> along with its size, would be returned to the caller.  However, the
>> function is no longer required in the IMA subsystem.  It previously served
>> to dump the measurement list during the kexec 'load' operation, but this
>> functionality has been replaced by an alternative approach in this patch
>> series.
>>
>> Remove the unused ima_dump_measurement_list function from the IMA
>> subsystem, to ensure a cleaner and more maintainable code.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Have you compiled the code and tested after applying each patch?
> Removing the caller before removing the code should have resulted in a
> warning.
>
Yes, I compiled each patch separately, and I did see this warning.
But removing ima_dump_measurement_list after implementing and
calling the alternate ima_allocate_buf_at_kexec_load() and
ima_populate_buf_at_kexec_execute() felt cleaner in terms of organizing
the patches.

I can remove the definition when I deprecate the usage, if we end up
using the alternate two functions.

~Tushar
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 224d88ccfe85..424930085c18 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -115,64 +115,6 @@  static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **
 }
 
 
-static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
-				     unsigned long segment_size)
-{
-	struct ima_queue_entry *qe;
-	struct seq_file file;
-	struct ima_kexec_hdr khdr;
-	int ret = 0;
-
-	/* segment size can't change between kexec load and execute */
-	file.buf = vmalloc(segment_size);
-	if (!file.buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	file.size = segment_size;
-	file.read_pos = 0;
-	file.count = sizeof(khdr);	/* reserved space */
-
-	memset(&khdr, 0, sizeof(khdr));
-	khdr.version = 1;
-	list_for_each_entry_rcu(qe, &ima_measurements, later) {
-		if (file.count < file.size) {
-			khdr.count++;
-			ima_measurements_show(&file, qe);
-		} else {
-			ret = -EINVAL;
-			break;
-		}
-	}
-
-	if (ret < 0)
-		goto out;
-
-	/*
-	 * fill in reserved space with some buffer details
-	 * (eg. version, buffer size, number of measurements)
-	 */
-	khdr.buffer_size = file.count;
-	if (ima_canonical_fmt) {
-		khdr.version = cpu_to_le16(khdr.version);
-		khdr.count = cpu_to_le64(khdr.count);
-		khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
-	}
-	memcpy(file.buf, &khdr, sizeof(khdr));
-
-	print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
-			     file.buf, file.count < 100 ? file.count : 100,
-			     true);
-
-	*buffer_size = file.count;
-	*buffer = file.buf;
-out:
-	if (ret == -EINVAL)
-		vfree(file.buf);
-	return ret;
-}
-
 /*
  * Called during kexec_file_load so that IMA can add a segment to the kexec
  * image for the measurement list for the next kernel.