| Message ID | 20231216010729.2904751-3-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ima: kexec: measure events between kexec load and execute | expand |
Hi Tushar, On Fri, 2023-12-15 at 17:07 -0800, Tushar Sugandhi wrote: > ima_dump_measurement_list() is called from ima_add_kexec_buffer() during > kexec 'load', which may result in loss of IMA measurements between kexec > 'load' and 'execute'. It needs to be called during kexec 'execute'. > > Implement ima_update_kexec_buffer(), to be called during kexec 'execute'. > Move ima_dump_measurement_list() function call from ima_add_kexec_buffer() > to ima_update_kexec_buffer(). Make the needed variables global for > accessibility during kexec 'load' and 'execute'. Implement and call > ima_measurements_suspend() and ima_measurements_resume() to help ensure > the integrity of the IMA log during copy. Add a reboot notifier_block to > trigger ima_update_kexec_buffer() during kexec soft-reboot. Exclude ima > segment from calculating and storing digest in function > kexec_calculate_store_digests(), since ima segment can be modified > after the digest is computed during kexec 'load'. > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> Wow! That's quite a bit for a single patch. This patch moves the ima_dump_measurement_list() call from kexec load to exec, but doesn't register the reboot notifier in this patch. I don't see how it is possible with just the previous and this patch applied that the measurement list is carried across kexec. Please test after applying each patch in the patch set to make sure that the measurement list is properly carried across kexec. Additional inline comments below. > --- > include/linux/kexec.h | 3 ++ > kernel/kexec_file.c | 8 ++++ > security/integrity/ima/ima.h | 2 + > security/integrity/ima/ima_kexec.c | 61 +++++++++++++++++++++++++----- > security/integrity/ima/ima_queue.c | 19 ++++++++++ > 5 files changed, 84 insertions(+), 9 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 22b5cd24f581..fd94404acc66 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -366,6 +366,9 @@ struct kimage { > > phys_addr_t ima_buffer_addr; > size_t ima_buffer_size; > + > + unsigned long ima_segment_index; > + bool is_ima_segment_index_set; > #endif > > /* Core ELF header buffer */ > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > index f989f5f1933b..bf758fd5062c 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image) > if (ksegment->kbuf == pi->purgatory_buf) > continue; > > + /* > + * Skip the segment if ima_segment_index is set and matches > + * the current index > + */ > + if (image->is_ima_segment_index_set && > + i == image->ima_segment_index) > + continue; With this change, the IMA segment is not included in the digest calculation, nor should it be included in the digest verification. However, I'm not seeing the matching code change in the digest verification. Please make ignoring the IMA segment a separate patch. > ret = crypto_shash_update(desc, ksegment->kbuf, > ksegment->bufsz); > if (ret) > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index c29db699c996..49a6047dd8eb 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -161,6 +161,8 @@ bool ima_template_has_modsig(const struct ima_template_desc *ima_template); > int ima_restore_measurement_entry(struct ima_template_entry *entry); > int ima_restore_measurement_list(loff_t bufsize, void *buf); > int ima_measurements_show(struct seq_file *m, void *v); > +void ima_measurements_suspend(void); > +void ima_measurements_resume(void); > unsigned long ima_get_binary_runtime_size(void); > int ima_init_template(void); > void ima_init_template_list(void); > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c > index ae27101d0615..0063d5e7b634 100644 > --- a/security/integrity/ima/ima_kexec.c > +++ b/security/integrity/ima/ima_kexec.c > @@ -16,6 +16,8 @@ > > #ifdef CONFIG_IMA_KEXEC > struct seq_file ima_kexec_file; > +static void *ima_kexec_buffer; > +static size_t kexec_segment_size; > > void ima_free_kexec_file_buf(struct seq_file *sf) > { > @@ -120,7 +122,6 @@ void ima_add_kexec_buffer(struct kimage *image) > /* use more understandable variable names than defined in kbuf */ > void *kexec_buffer = NULL; > size_t kexec_buffer_size; > - size_t kexec_segment_size; > int ret; > > /* > @@ -145,17 +146,10 @@ void ima_add_kexec_buffer(struct kimage *image) > return; > } > > - ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer, > - kexec_segment_size); > - if (ret < 0) { > - pr_err("%s: Failed to dump IMA measurements. Error:%d.\n", > - __func__, ret); > - return; > - } > - > kbuf.buffer = kexec_buffer; > kbuf.bufsz = kexec_buffer_size; > kbuf.memsz = kexec_segment_size; > + image->is_ima_segment_index_set = false; > ret = kexec_add_buffer(&kbuf); > if (ret) { > pr_err("Error passing over kexec measurement buffer.\n"); > @@ -166,10 +160,59 @@ void ima_add_kexec_buffer(struct kimage *image) > image->ima_buffer_addr = kbuf.mem; > image->ima_buffer_size = kexec_segment_size; > image->ima_buffer = kexec_buffer; > + image->ima_segment_index = image->nr_segments - 1; > + image->is_ima_segment_index_set = true; > > pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n", > kbuf.mem); > } > + > +/* > + * Called during kexec execute so that IMA can update the measurement list. > + */ > +static int ima_update_kexec_buffer(struct notifier_block *self, > + unsigned long action, void *data) > +{ > + void *buf = NULL; > + size_t buf_size; > + bool resume = false; > + int ret = NOTIFY_OK; > + > + if (!kexec_in_progress) { > + pr_info("%s: No kexec in progress.\n", __func__); > + return ret; > + } > + > + if (!ima_kexec_buffer) { > + pr_err("%s: Kexec buffer not set.\n", __func__); > + return ret; > + } > + > + ima_measurements_suspend(); > + > + ret = ima_dump_measurement_list(&buf_size, &buf, > + kexec_segment_size); > + > + if (!buf) { > + pr_err("%s: Dump measurements failed. Error:%d\n", > + __func__, ret); > + resume = true; > + goto out; > + } > + memcpy(ima_kexec_buffer, buf, buf_size); > +out: > + ima_kexec_buffer = NULL; > + > + if (resume) > + ima_measurements_resume(); > + > + return ret; > +} > + > +struct notifier_block update_buffer_nb = { > + .notifier_call = ima_update_kexec_buffer, > +}; > + > #endif /* IMA_KEXEC */ > > /* > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c > index 532da87ce519..cb9abc02a304 100644 > --- a/security/integrity/ima/ima_queue.c > +++ b/security/integrity/ima/ima_queue.c > @@ -44,6 +44,11 @@ struct ima_h_table ima_htable = { > */ > static DEFINE_MUTEX(ima_extend_list_mutex); > > +/* > + * Used internally by the kernel to suspend-resume ima measurements. > + */ > +static atomic_t suspend_ima_measurements; > + > /* lookup up the digest value in the hash table, and return the entry */ > static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value, > int pcr) > @@ -148,6 +153,20 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr) > return result; > } > > +void ima_measurements_suspend(void) > +{ > + mutex_lock(&ima_extend_list_mutex); > + atomic_set(&suspend_ima_measurements, 1); > + mutex_unlock(&ima_extend_list_mutex); > +} > + > +void ima_measurements_resume(void) > +{ > + mutex_lock(&ima_extend_list_mutex); > + atomic_set(&suspend_ima_measurements, 0); > + mutex_unlock(&ima_extend_list_mutex); > +} > + Suspending and resuming extending the measurement list should be a separate patch as well, with its own patch description. > /* > * Add template entry to the measurement list and hash table, and > * extend the pcr.
Apologies for the late response on this particular patch (v3 2/7) Mimi. I was on vacation in December. I was meaning to respond to this one when I came back, but I was caught in between other work items last few days. Sorry if it caused any confusion. Responses below. On 12/20/23 11:02, Mimi Zohar wrote: > Hi Tushar, > > On Fri, 2023-12-15 at 17:07 -0800, Tushar Sugandhi wrote: >> ima_dump_measurement_list() is called from ima_add_kexec_buffer() during >> kexec 'load', which may result in loss of IMA measurements between kexec >> 'load' and 'execute'. It needs to be called during kexec 'execute'. >> >> Implement ima_update_kexec_buffer(), to be called during kexec 'execute'. >> Move ima_dump_measurement_list() function call from ima_add_kexec_buffer() >> to ima_update_kexec_buffer(). Make the needed variables global for >> accessibility during kexec 'load' and 'execute'. Implement and call >> ima_measurements_suspend() and ima_measurements_resume() to help ensure >> the integrity of the IMA log during copy. Add a reboot notifier_block to >> trigger ima_update_kexec_buffer() during kexec soft-reboot. Exclude ima >> segment from calculating and storing digest in function >> kexec_calculate_store_digests(), since ima segment can be modified >> after the digest is computed during kexec 'load'. >> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > > Wow! That's quite a bit for a single patch. > > This patch moves the ima_dump_measurement_list() call from kexec load > to exec, but doesn't register the reboot notifier in this patch. I > don't see how it is possible with just the previous and this patch > applied that the measurement list is carried across kexec. Ah. That's a good catch. I was only checking if I can boot into the Kernel for testing bisect-safe readiness for each patch. I will ensure the move of ima_dump_measurement_list() and registering the reboot notifier at execute stays an atomic operation in a single patch. > > Please test after applying each patch in the patch set to make sure > that the measurement list is properly carried across kexec. > Yup. I was only checking if I can boot into the Kernel after each patch. My bad. :( Going forward, I will check each patch for the measurement list carry over after kexec. > Additional inline comments below. > >> --- >> include/linux/kexec.h | 3 ++ >> kernel/kexec_file.c | 8 ++++ >> security/integrity/ima/ima.h | 2 + >> security/integrity/ima/ima_kexec.c | 61 +++++++++++++++++++++++++----- >> security/integrity/ima/ima_queue.c | 19 ++++++++++ >> 5 files changed, 84 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >> index 22b5cd24f581..fd94404acc66 100644 >> --- a/include/linux/kexec.h >> +++ b/include/linux/kexec.h >> @@ -366,6 +366,9 @@ struct kimage { >> >> phys_addr_t ima_buffer_addr; >> size_t ima_buffer_size; >> + >> + unsigned long ima_segment_index; >> + bool is_ima_segment_index_set; >> #endif >> >> /* Core ELF header buffer */ >> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c >> index f989f5f1933b..bf758fd5062c 100644 >> --- a/kernel/kexec_file.c >> +++ b/kernel/kexec_file.c >> @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image) >> if (ksegment->kbuf == pi->purgatory_buf) >> continue; >> >> + /* >> + * Skip the segment if ima_segment_index is set and matches >> + * the current index >> + */ >> + if (image->is_ima_segment_index_set && >> + i == image->ima_segment_index) >> + continue; > > With this change, the IMA segment is not included in the digest > calculation, nor should it be included in the digest verification. > However, I'm not seeing the matching code change in the digest > verification. > Fair question. But I don't think anything else needs to be done here. The way kexec_calculate_store_digests() and verify_sha256_digest() are implemented, it already skips verification of the segments if the segment is not part of 'purgatory_sha_regions'. In kexec_calculate_store_digests(), my change is to 'continue' when the segment is the IMA segment when the function is going through all the segments in a for loop [1]. Therefore in kexec_calculate_store_digests() - - crypto_shash_update() is not called for IMA segment [1]. - sha_regions[j] is not updated with IMA segment [1]. - This 'sha_regions' variable later becomes 'purgatory_sha_regions' in kexec_calculate_store_digests [1]. - and verify_sha256_digest() only verifies 'purgatory_sha_regions'[2]. Since IMA segment is not part of the 'purgatory_sha_regions', it is not included in the verification as part of verify_sha256_digest(). I have pasted the relevant code below for quick reference [1][2]. > Please make ignoring the IMA segment a separate patch. > Sure. Will do. >> ret = crypto_shash_update(desc, ksegment->kbuf, >> ksegment->bufsz); >> if (ret) ... ... ... >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h >> index c29db699c996..49a6047dd8eb 100644 > > Suspending and resuming extending the measurement list should be a > separate patch as well, with its own patch description. > Sure. Will do. >> /* >> * Add template entry to the measurement list and hash table, and >> * extend the pcr. > ---------------------------------------------------------------------------------- Reference code in the context of kexec_calculate_store_digests() and verify_sha256_digest() conversation above. ---------------------------------------------------------------------------------- [1] https://lore.kernel.org/all/20231216010729.2904751-3-tusharsu@linux.microsoft.com/ /* Calculate and store the digest of segments */ static int kexec_calculate_store_digests(struct kimage *image) { ... ... for (j = i = 0; i < image->nr_segments; i++) { struct kexec_segment *ksegment; ksegment = &image->segment[i]; /* * Skip purgatory as it will be modified once we put digest * info in purgatory. */ if (ksegment->kbuf == pi->purgatory_buf) continue; + /* + * Skip the segment if ima_segment_index is set and matches + * the current index + */ + if (image->is_ima_segment_index_set && + i == image->ima_segment_index) + continue; + ret = crypto_shash_update(desc, ksegment->kbuf, ksegment->bufsz); if (ret) break; /* * Assume rest of the buffer is filled with zero and * update digest accordingly. */ nullsz = ksegment->memsz - ksegment->bufsz; while (nullsz) { unsigned long bytes = nullsz; if (bytes > zero_buf_sz) bytes = zero_buf_sz; ret = crypto_shash_update(desc, zero_buf, bytes); if (ret) break; nullsz -= bytes; } if (ret) break; sha_regions[j].start = ksegment->mem; sha_regions[j].len = ksegment->memsz; j++; } if (!ret) { ret = crypto_shash_final(desc, digest); if (ret) goto out_free_digest; ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions", sha_regions, sha_region_sz, 0); if (ret) goto out_free_digest; ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha256_digest", digest, SHA256_DIGEST_SIZE, 0); if (ret) goto out_free_digest; } out_free_digest: kfree(digest); out_free_sha_regions: vfree(sha_regions); out_free_desc: kfree(desc); out_free_tfm: kfree(tfm); out: return ret; } --------------------------------------------------------------------------- [2] https://elixir.bootlin.com/linux/latest/source/arch/x86/purgatory/purgatory.c#L24 u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(".kexec-purgatory"); struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX] __section(".kexec-purgatory"); static int verify_sha256_digest(void) { struct kexec_sha_region *ptr, *end; u8 digest[SHA256_DIGEST_SIZE]; struct sha256_state sctx; sha256_init(&sctx); end = purgatory_sha_regions + ARRAY_SIZE(purgatory_sha_regions); for (ptr = purgatory_sha_regions; ptr < end; ptr++) sha256_update(&sctx, (uint8_t *)(ptr->start), ptr->len); sha256_final(&sctx, digest); if (memcmp(digest, purgatory_sha256_digest, sizeof(digest))) return 1; return 0; } Thanks, Tushar
Hi Tushar, > > This patch moves the ima_dump_measurement_list() call from kexec load > > to exec, but doesn't register the reboot notifier in this patch. I > > don't see how it is possible with just the previous and this patch > > applied that the measurement list is carried across kexec. > Ah. That's a good catch. > I was only checking if I can boot into the Kernel for testing > bisect-safe readiness for each patch. I will ensure the move of > ima_dump_measurement_list() and registering the reboot notifier at > execute stays an atomic operation in a single patch. Thanks! > >> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > >> index f989f5f1933b..bf758fd5062c 100644 > >> --- a/kernel/kexec_file.c > >> +++ b/kernel/kexec_file.c > >> @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image) > >> if (ksegment->kbuf == pi->purgatory_buf) > >> continue; > >> > >> + /* > >> + * Skip the segment if ima_segment_index is set and matches > >> + * the current index > >> + */ > >> + if (image->is_ima_segment_index_set && > >> + i == image->ima_segment_index) > >> + continue; > > > > With this change, the IMA segment is not included in the digest > > calculation, nor should it be included in the digest verification. > > However, I'm not seeing the matching code change in the digest > > verification. > > > Fair question. > > But I don't think anything else needs to be done here. > > The way kexec_calculate_store_digests() and verify_sha256_digest() > are implemented, it already skips verification of the segments if > the segment is not part of 'purgatory_sha_regions'. > > In kexec_calculate_store_digests(), my change is to 'continue' when the > segment is the IMA segment when the function is going through all the > segments in a for loop [1]. > > Therefore in kexec_calculate_store_digests() - > - crypto_shash_update() is not called for IMA segment [1]. > - sha_regions[j] is not updated with IMA segment [1]. > - This 'sha_regions' variable later becomes 'purgatory_sha_regions' > in kexec_calculate_store_digests [1]. > - and verify_sha256_digest() only verifies 'purgatory_sha_regions'[2]. > > Since IMA segment is not part of the 'purgatory_sha_regions', it is > not included in the verification as part of verify_sha256_digest(). > > > Please make ignoring the IMA segment a separate patch. > > > Sure. Will do. Thank you for the explanation. Please include in the patch description a statement about the "sha_regions" not including the IMA segment, so nothing is needed on the verify side. > > >> ret = crypto_shash_update(desc, ksegment->kbuf, > >> ksegment->bufsz); > >> if (ret) > ... > ... > ... > >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > >> index c29db699c996..49a6047dd8eb 100644 > > > > Suspending and resuming extending the measurement list should be a > > separate patch as well, with its own patch description. > > > Sure. Will do. Thanks! Mimi
On 1/12/24 09:06, Mimi Zohar wrote: > >>>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c >>>> index f989f5f1933b..bf758fd5062c 100644 >>>> --- a/kernel/kexec_file.c >>>> +++ b/kernel/kexec_file.c >>>> @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image) >>>> if (ksegment->kbuf == pi->purgatory_buf) >>>> continue; >>>> >>>> + /* >>>> + * Skip the segment if ima_segment_index is set and matches >>>> + * the current index >>>> + */ >>>> + if (image->is_ima_segment_index_set && >>>> + i == image->ima_segment_index) >>>> + continue; >>> With this change, the IMA segment is not included in the digest >>> calculation, nor should it be included in the digest verification. >>> However, I'm not seeing the matching code change in the digest >>> verification. >>> >> Fair question. >> >> But I don't think anything else needs to be done here. >> >> The way kexec_calculate_store_digests() and verify_sha256_digest() >> are implemented, it already skips verification of the segments if >> the segment is not part of 'purgatory_sha_regions'. >> >> In kexec_calculate_store_digests(), my change is to 'continue' when the >> segment is the IMA segment when the function is going through all the >> segments in a for loop [1]. >> >> Therefore in kexec_calculate_store_digests() - >> - crypto_shash_update() is not called for IMA segment [1]. >> - sha_regions[j] is not updated with IMA segment [1]. >> - This 'sha_regions' variable later becomes 'purgatory_sha_regions' >> in kexec_calculate_store_digests [1]. >> - and verify_sha256_digest() only verifies 'purgatory_sha_regions'[2]. >> >> Since IMA segment is not part of the 'purgatory_sha_regions', it is >> not included in the verification as part of verify_sha256_digest(). >> >>> Please make ignoring the IMA segment a separate patch. >>> >> Sure. Will do. > Thank you for the explanation. Please include in the patch description a > statement about the "sha_regions" not including the IMA segment, so nothing is > needed on the verify side. Definitely. Will do.
diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 22b5cd24f581..fd94404acc66 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -366,6 +366,9 @@ struct kimage { phys_addr_t ima_buffer_addr; size_t ima_buffer_size; + + unsigned long ima_segment_index; + bool is_ima_segment_index_set; #endif /* Core ELF header buffer */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index f989f5f1933b..bf758fd5062c 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image) if (ksegment->kbuf == pi->purgatory_buf) continue; + /* + * Skip the segment if ima_segment_index is set and matches + * the current index + */ + if (image->is_ima_segment_index_set && + i == image->ima_segment_index) + continue; + ret = crypto_shash_update(desc, ksegment->kbuf, ksegment->bufsz); if (ret) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index c29db699c996..49a6047dd8eb 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -161,6 +161,8 @@ bool ima_template_has_modsig(const struct ima_template_desc *ima_template); int ima_restore_measurement_entry(struct ima_template_entry *entry); int ima_restore_measurement_list(loff_t bufsize, void *buf); int ima_measurements_show(struct seq_file *m, void *v); +void ima_measurements_suspend(void); +void ima_measurements_resume(void); unsigned long ima_get_binary_runtime_size(void); int ima_init_template(void); void ima_init_template_list(void); diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index ae27101d0615..0063d5e7b634 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -16,6 +16,8 @@ #ifdef CONFIG_IMA_KEXEC struct seq_file ima_kexec_file; +static void *ima_kexec_buffer; +static size_t kexec_segment_size; void ima_free_kexec_file_buf(struct seq_file *sf) { @@ -120,7 +122,6 @@ void ima_add_kexec_buffer(struct kimage *image) /* use more understandable variable names than defined in kbuf */ void *kexec_buffer = NULL; size_t kexec_buffer_size; - size_t kexec_segment_size; int ret; /* @@ -145,17 +146,10 @@ void ima_add_kexec_buffer(struct kimage *image) return; } - ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer, - kexec_segment_size); - if (ret < 0) { - pr_err("%s: Failed to dump IMA measurements. Error:%d.\n", - __func__, ret); - return; - } - kbuf.buffer = kexec_buffer; kbuf.bufsz = kexec_buffer_size; kbuf.memsz = kexec_segment_size; + image->is_ima_segment_index_set = false; ret = kexec_add_buffer(&kbuf); if (ret) { pr_err("Error passing over kexec measurement buffer.\n"); @@ -166,10 +160,59 @@ void ima_add_kexec_buffer(struct kimage *image) image->ima_buffer_addr = kbuf.mem; image->ima_buffer_size = kexec_segment_size; image->ima_buffer = kexec_buffer; + image->ima_segment_index = image->nr_segments - 1; + image->is_ima_segment_index_set = true; pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n", kbuf.mem); } + +/* + * Called during kexec execute so that IMA can update the measurement list. + */ +static int ima_update_kexec_buffer(struct notifier_block *self, + unsigned long action, void *data) +{ + void *buf = NULL; + size_t buf_size; + bool resume = false; + int ret = NOTIFY_OK; + + if (!kexec_in_progress) { + pr_info("%s: No kexec in progress.\n", __func__); + return ret; + } + + if (!ima_kexec_buffer) { + pr_err("%s: Kexec buffer not set.\n", __func__); + return ret; + } + + ima_measurements_suspend(); + + ret = ima_dump_measurement_list(&buf_size, &buf, + kexec_segment_size); + + if (!buf) { + pr_err("%s: Dump measurements failed. Error:%d\n", + __func__, ret); + resume = true; + goto out; + } + memcpy(ima_kexec_buffer, buf, buf_size); +out: + ima_kexec_buffer = NULL; + + if (resume) + ima_measurements_resume(); + + return ret; +} + +struct notifier_block update_buffer_nb = { + .notifier_call = ima_update_kexec_buffer, +}; + #endif /* IMA_KEXEC */ /* diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index 532da87ce519..cb9abc02a304 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -44,6 +44,11 @@ struct ima_h_table ima_htable = { */ static DEFINE_MUTEX(ima_extend_list_mutex); +/* + * Used internally by the kernel to suspend-resume ima measurements. + */ +static atomic_t suspend_ima_measurements; + /* lookup up the digest value in the hash table, and return the entry */ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value, int pcr) @@ -148,6 +153,20 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr) return result; } +void ima_measurements_suspend(void) +{ + mutex_lock(&ima_extend_list_mutex); + atomic_set(&suspend_ima_measurements, 1); + mutex_unlock(&ima_extend_list_mutex); +} + +void ima_measurements_resume(void) +{ + mutex_lock(&ima_extend_list_mutex); + atomic_set(&suspend_ima_measurements, 0); + mutex_unlock(&ima_extend_list_mutex); +} + /* * Add template entry to the measurement list and hash table, and * extend the pcr.
ima_dump_measurement_list() is called from ima_add_kexec_buffer() during kexec 'load', which may result in loss of IMA measurements between kexec 'load' and 'execute'. It needs to be called during kexec 'execute'. Implement ima_update_kexec_buffer(), to be called during kexec 'execute'. Move ima_dump_measurement_list() function call from ima_add_kexec_buffer() to ima_update_kexec_buffer(). Make the needed variables global for accessibility during kexec 'load' and 'execute'. Implement and call ima_measurements_suspend() and ima_measurements_resume() to help ensure the integrity of the IMA log during copy. Add a reboot notifier_block to trigger ima_update_kexec_buffer() during kexec soft-reboot. Exclude ima segment from calculating and storing digest in function kexec_calculate_store_digests(), since ima segment can be modified after the digest is computed during kexec 'load'. Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> --- include/linux/kexec.h | 3 ++ kernel/kexec_file.c | 8 ++++ security/integrity/ima/ima.h | 2 + security/integrity/ima/ima_kexec.c | 61 +++++++++++++++++++++++++----- security/integrity/ima/ima_queue.c | 19 ++++++++++ 5 files changed, 84 insertions(+), 9 deletions(-)