| Message ID | 20230703215709.1195644-3-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: > 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? > > 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@linux.microsoft.com> > --- > 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) > {
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@linux.microsoft.com> >> --- >> 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) >> {
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; +} + + static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer, unsigned long segment_size) {
There is no existing IMA functionality to just populate the buffer at kexec execute with IMA measurements. 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@linux.microsoft.com> --- security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)