| Message ID | 20230703215709.1195644-2-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: measure events between kexec load and execute | expand |
Hi Tushar, On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: > IMA does not provide a mechanism to allocate memory for IMA log storage > during kexec operation. The IMA measurement list is currently being carried across kexec, so obviously a buffer is being allocated for it. IMA not allocating memory for the measurment list is not the problem statement. Please concisely provide the problem statement, explaining why IMA needs to allocate the buffer. > The function should handle the scenario where > the kexec load is called multiple times. Currently the buffer is being freed with the kexec 'unload'. With this patch IMA is allocating a buffer for the measurement list, which needs to be freed independently of the kexec 'unload'. > Implement a function to allocate buffer of size kexec_segment_size at > kexec load. If the buffer was already allocated, free that buffer and > reallocate. Finally, initialihze ima_khdr struct. > > The patch operates under the assumption that the segment size does not > change between kexec load and execute. > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
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: >> IMA does not provide a mechanism to allocate memory for IMA log storage >> during kexec operation. > The IMA measurement list is currently being carried across kexec, so > obviously a buffer is being allocated for it. IMA not allocating > memory for the measurment list is not the problem statement. Please > concisely provide the problem statement, explaining why IMA needs to > allocate the buffer. > I meant IMA does not provide separate functions to allocate buffer and populate measurements. Both operations are wrapped in an atomic ima_dump_measurement_list(). As I mentioned in the comment in the cover letter, if there is no such technical limitation to allocate the buffer and copy the measurements at kexec ‘execute’ – I will make the necessary code changes and update the above line in the patch description accordingly. >> The function should handle the scenario where >> the kexec load is called multiple times. > Currently the buffer is being freed with the kexec 'unload'. With this > patch IMA is allocating a buffer for the measurement list, which needs > to be freed independently of the kexec 'unload'. If we end up allocating the buffer at kexec ‘execute’ (which results in soft boot to next Kernel) – is it technically possible that kexec ‘unload’ being called after calling kexec ‘execute’? If not, should I still free the buffer at kexec ‘unload’ in this scenario? ~Tushar >> Implement a function to allocate buffer of size kexec_segment_size at >> kexec load. If the buffer was already allocated, free that buffer and >> reallocate. Finally, initialihze ima_khdr struct. >> >> The patch operates under the assumption that the segment size does not >> change between kexec load and execute. >> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote: > 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: > >> IMA does not provide a mechanism to allocate memory for IMA log storage > >> during kexec operation. > > The IMA measurement list is currently being carried across kexec, so > > obviously a buffer is being allocated for it. IMA not allocating > > memory for the measurment list is not the problem statement. Please > > concisely provide the problem statement, explaining why IMA needs to > > allocate the buffer. > > > I meant IMA does not provide separate functions to allocate buffer and > populate measurements. Both operations are wrapped in an atomic > ima_dump_measurement_list(). Ok. > As I mentioned in the comment in the cover letter, if there is no such > technical limitation to allocate the buffer and copy the measurements at > kexec ‘execute’ – I will make the necessary code changes and update the > above line in the patch description accordingly. The "normal" way of making this type of change would be to split the existing ima_dump_measurement_list() function. Copying the measurement list would still be named ima_dump_measurement_list(). The other could be named ima_alloc_kexec_buf(). Both functions initially would be called. Eric, besides updating the buffer at kexec execute, is there anything else that needs to be done (e.g. updating digests)? > >> The function should handle the scenario where > >> the kexec load is called multiple times. > > Currently the buffer is being freed with the kexec 'unload'. With this > > patch IMA is allocating a buffer for the measurement list, which needs > > to be freed independently of the kexec 'unload'. > If we end up allocating the buffer at kexec ‘execute’ (which results in > soft boot to next Kernel) – is it technically possible that > kexec ‘unload’ being called after calling kexec ‘execute’? > If not, should I still free the buffer at kexec ‘unload’ in this > scenario? The question is how to access the buffer once kexec_add_buffer() is called. Mimi
On 7/11/23 14:11, Mimi Zohar wrote: > On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote: >> 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: >>>> IMA does not provide a mechanism to allocate memory for IMA log storage >>>> during kexec operation. >>> The IMA measurement list is currently being carried across kexec, so >>> obviously a buffer is being allocated for it. IMA not allocating >>> memory for the measurment list is not the problem statement. Please >>> concisely provide the problem statement, explaining why IMA needs to >>> allocate the buffer. >>> >> I meant IMA does not provide separate functions to allocate buffer and >> populate measurements. Both operations are wrapped in an atomic >> ima_dump_measurement_list(). > Ok. > >> As I mentioned in the comment in the cover letter, if there is no such >> technical limitation to allocate the buffer and copy the measurements at >> kexec ‘execute’ – I will make the necessary code changes and update the >> above line in the patch description accordingly. > The "normal" way of making this type of change would be to split the > existing ima_dump_measurement_list() function. Copying the measurement > list would still be named ima_dump_measurement_list(). The other could > be named ima_alloc_kexec_buf(). Both functions initially would be > called. > Sounds good. I will make that change. I will define ima_alloc_kexec_buf() to allocate memory at kexec 'load'. And update ima_dump_measurement_list() to only copy the measurements. Both the functions will be called during kexec 'load'. And only the updated ima_dump_measurement_list() will be called during kexec 'execute'. Please correct me if I misunderstood. BTW, as discussed elsewhere I am hoping to get clarity on if we can move everything (memory allocation and copying measurements) to kexec 'execute'. My current understanding is segment mapping must happen at kexec 'load'. Hopefully someone on this thread can validate if its true or not. > Eric, besides updating the buffer at kexec execute, is there anything > else that needs to be done (e.g. updating digests)? > I will also wait for his response. :) >>>> The function should handle the scenario where >>>> the kexec load is called multiple times. >>> Currently the buffer is being freed with the kexec 'unload'. With this >>> patch IMA is allocating a buffer for the measurement list, which needs >>> to be freed independently of the kexec 'unload'. >> If we end up allocating the buffer at kexec ‘execute’ (which results in >> soft boot to next Kernel) – is it technically possible that >> kexec ‘unload’ being called after calling kexec ‘execute’? >> If not, should I still free the buffer at kexec ‘unload’ in this >> scenario? > The question is how to access the buffer once kexec_add_buffer() is > called. > > Mimi
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 419dc405c831..48a683874044 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -15,6 +15,53 @@ #include "ima.h" #ifdef CONFIG_IMA_KEXEC +struct seq_file ima_kexec_file; +struct ima_kexec_hdr ima_khdr; +static size_t kexec_segment_size; + +void ima_clear_kexec_file(void) +{ + vfree(ima_kexec_file.buf); + ima_kexec_file.buf = NULL; + ima_kexec_file.size = 0; + ima_kexec_file.read_pos = 0; + ima_kexec_file.count = 0; +} + +static int ima_allocate_buf_at_kexec_load(void) +{ + if ((kexec_segment_size == 0) || + (kexec_segment_size == ULONG_MAX) || + ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) { + pr_err("%s: Invalid segment size for kexec: %zu\n", + __func__, kexec_segment_size); + return -EINVAL; + } + + /* if kexec load was called before, clear the existing buffer + * before allocating a new one + */ + if (ima_kexec_file.buf) + ima_clear_kexec_file(); + + /* segment size can't change between kexec load and execute */ + ima_kexec_file.buf = vmalloc(kexec_segment_size); + if (!ima_kexec_file.buf) { + pr_err("%s: No memory for ima kexec measurement buffer\n", + __func__); + return -ENOMEM; + } + + ima_kexec_file.size = kexec_segment_size; + ima_kexec_file.read_pos = 0; + ima_kexec_file.count = sizeof(ima_khdr); /* reserved space */ + + memset(&ima_khdr, 0, sizeof(ima_khdr)); + ima_khdr.version = 1; + + return 0; +} + static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer, unsigned long segment_size) {
IMA does not provide a mechanism to allocate memory for IMA log storage during kexec operation. The function should handle the scenario where the kexec load is called multiple times. Implement a function to allocate buffer of size kexec_segment_size at kexec load. If the buffer was already allocated, free that buffer and reallocate. Finally, initialize ima_khdr struct. The patch operates under the assumption that the segment size does not change between kexec load and execute. Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> --- security/integrity/ima/ima_kexec.c | 47 ++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)