| Message ID | 20250416021028.1403-5-chenste@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ima: kexec: measure events between kexec load and execute | expand |
On 04/15/25 at 07:10pm, steven chen wrote: > From: Steven Chen <chenste@linux.microsoft.com> > > Currently, the function kexec_calculate_store_digests() calculates and > stores the digest of the segment during the kexec_file_load syscall, > where the IMA segment is also allocated. > > Later, the IMA segment will be updated with the measurement log at the > kexec execute stage when a kexec reboot is initiated. Therefore, the > digests should be updated for the IMA segment in the normal case. The > problem is that the content of memory segments carried over to the new > kernel during the kexec systemcall can be changed at kexec 'execute' > stage, but the size and the location of the memory segments cannot be > changed at kexec 'execute' stage. > > To address this, skip the calculation and storage of the digest for the > IMA segment in kexec_calculate_store_digests() so that it is not added > to the purgatory_sha_regions. > > With this change, the IMA segment is not included in the digest > calculation, storage, and verification. > > Cc: Eric Biederman <ebiederm@xmission.com> > Cc: Baoquan He <bhe@redhat.com> > Cc: Vivek Goyal <vgoyal@redhat.com> > Cc: Dave Young <dyoung@redhat.com> > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: Steven Chen <chenste@linux.microsoft.com> You may need to set tags as below for this patch: Co-developed-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> Signed-off-by: Steven Chen <chenste@linux.microsoft.com> =======Quoted from Documentation/process/5.Posting.rst===== - Co-developed-by: states that the patch was co-created by several developers; it is a used to give attribution to co-authors (in addition to the author attributed by the From: tag) when multiple people work on a single patch. Every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author. Details and examples can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`. ======== Other than this nit, this looks good to me: Acked-by: Baoquan He <bhe@redhat.com> > --- > include/linux/kexec.h | 3 +++ > kernel/kexec_file.c | 22 ++++++++++++++++++++++ > security/integrity/ima/ima_kexec.c | 3 +++ > 3 files changed, 28 insertions(+) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 7d6b12f8b8d0..107e726f2ef3 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -362,6 +362,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 3eedb8c226ad..606132253c79 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void) > } > #endif > > +#ifdef CONFIG_IMA_KEXEC > +static bool check_ima_segment_index(struct kimage *image, int i) > +{ > + if (image->is_ima_segment_index_set && i == image->ima_segment_index) > + return true; > + else > + return false; > +} > +#else > +static bool check_ima_segment_index(struct kimage *image, int i) > +{ > + return false; > +} > +#endif > + > static int kexec_calculate_store_digests(struct kimage *image); > > /* Maximum size in bytes for kernel/initrd files. */ > @@ -764,6 +779,13 @@ 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 (check_ima_segment_index(image, i)) > + continue; > + > ret = crypto_shash_update(desc, ksegment->kbuf, > ksegment->bufsz); > if (ret) > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c > index b12ac3619b8f..7e0a19c3483f 100644 > --- a/security/integrity/ima/ima_kexec.c > +++ b/security/integrity/ima/ima_kexec.c > @@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image) > 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"); > @@ -155,6 +156,8 @@ 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; > > kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n", > kbuf.mem); > -- > 2.43.0 >
On 4/18/2025 12:37 AM, Baoquan He wrote: > On 04/15/25 at 07:10pm, steven chen wrote: >> From: Steven Chen <chenste@linux.microsoft.com> >> >> Currently, the function kexec_calculate_store_digests() calculates and >> stores the digest of the segment during the kexec_file_load syscall, >> where the IMA segment is also allocated. >> >> Later, the IMA segment will be updated with the measurement log at the >> kexec execute stage when a kexec reboot is initiated. Therefore, the >> digests should be updated for the IMA segment in the normal case. The >> problem is that the content of memory segments carried over to the new >> kernel during the kexec systemcall can be changed at kexec 'execute' >> stage, but the size and the location of the memory segments cannot be >> changed at kexec 'execute' stage. >> >> To address this, skip the calculation and storage of the digest for the >> IMA segment in kexec_calculate_store_digests() so that it is not added >> to the purgatory_sha_regions. >> >> With this change, the IMA segment is not included in the digest >> calculation, storage, and verification. >> >> Cc: Eric Biederman <ebiederm@xmission.com> >> Cc: Baoquan He <bhe@redhat.com> >> Cc: Vivek Goyal <vgoyal@redhat.com> >> Cc: Dave Young <dyoung@redhat.com> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> >> Signed-off-by: Steven Chen <chenste@linux.microsoft.com> > You may need to set tags as below for this patch: > > Co-developed-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: Steven Chen <chenste@linux.microsoft.com> > > =======Quoted from Documentation/process/5.Posting.rst===== > - Co-developed-by: states that the patch was co-created by several developers; > it is a used to give attribution to co-authors (in addition to the author > attributed by the From: tag) when multiple people work on a single patch. > Every Co-developed-by: must be immediately followed by a Signed-off-by: of > the associated co-author. Details and examples can be found in > :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`. > ======== > > Other than this nit, this looks good to me: > > Acked-by: Baoquan He <bhe@redhat.com> Hi Baoquan, I will add Co-developed-by tag in next version. Thanks, Steven >> --- >> include/linux/kexec.h | 3 +++ >> kernel/kexec_file.c | 22 ++++++++++++++++++++++ >> security/integrity/ima/ima_kexec.c | 3 +++ >> 3 files changed, 28 insertions(+) >> >> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >> index 7d6b12f8b8d0..107e726f2ef3 100644 >> --- a/include/linux/kexec.h >> +++ b/include/linux/kexec.h >> @@ -362,6 +362,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 3eedb8c226ad..606132253c79 100644 >> --- a/kernel/kexec_file.c >> +++ b/kernel/kexec_file.c >> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void) >> } >> #endif >> >> +#ifdef CONFIG_IMA_KEXEC >> +static bool check_ima_segment_index(struct kimage *image, int i) >> +{ >> + if (image->is_ima_segment_index_set && i == image->ima_segment_index) >> + return true; >> + else >> + return false; >> +} >> +#else >> +static bool check_ima_segment_index(struct kimage *image, int i) >> +{ >> + return false; >> +} >> +#endif >> + >> static int kexec_calculate_store_digests(struct kimage *image); >> >> /* Maximum size in bytes for kernel/initrd files. */ >> @@ -764,6 +779,13 @@ 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 (check_ima_segment_index(image, i)) >> + continue; >> + >> ret = crypto_shash_update(desc, ksegment->kbuf, >> ksegment->bufsz); >> if (ret) >> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c >> index b12ac3619b8f..7e0a19c3483f 100644 >> --- a/security/integrity/ima/ima_kexec.c >> +++ b/security/integrity/ima/ima_kexec.c >> @@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image) >> 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"); >> @@ -155,6 +156,8 @@ 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; >> >> kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n", >> kbuf.mem); >> -- >> 2.43.0 >>
diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 7d6b12f8b8d0..107e726f2ef3 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -362,6 +362,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 3eedb8c226ad..606132253c79 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void) } #endif +#ifdef CONFIG_IMA_KEXEC +static bool check_ima_segment_index(struct kimage *image, int i) +{ + if (image->is_ima_segment_index_set && i == image->ima_segment_index) + return true; + else + return false; +} +#else +static bool check_ima_segment_index(struct kimage *image, int i) +{ + return false; +} +#endif + static int kexec_calculate_store_digests(struct kimage *image); /* Maximum size in bytes for kernel/initrd files. */ @@ -764,6 +779,13 @@ 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 (check_ima_segment_index(image, i)) + continue; + ret = crypto_shash_update(desc, ksegment->kbuf, ksegment->bufsz); if (ret) diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index b12ac3619b8f..7e0a19c3483f 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image) 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"); @@ -155,6 +156,8 @@ 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; kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n", kbuf.mem);