Messages in this thread | | | Subject | Re: [PATCH v5 2/7] IMA: update process_buffer_measurement to measure buffer hash | From | Tushar Sugandhi <> | Date | Thu, 12 Nov 2020 13:48:53 -0800 |
| |
On 2020-11-06 4:11 a.m., Mimi Zohar wrote: > Hi Tushar, > > Below inline are a few additional comments. > >> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c >> index ae5da9f3339d..4485d87c0aa5 100644 >> --- a/security/integrity/ima/ima_main.c >> +++ b/security/integrity/ima/ima_main.c >> @@ -787,12 +787,15 @@ int ima_post_load_data(char *buf, loff_t size, >> * @func: IMA hook >> * @pcr: pcr to extend the measurement >> * @func_data: private data specific to @func, can be NULL. >> + * @measure_buf_hash: if set to true - will measure hash of the buf, >> + * instead of buf >> * >> * Based on policy, the buffer is measured into the ima log. > > Both the brief and longer function descriptions need to be updated, as > well as the last argument description. The last argument should be > limited to "measure buffer hash". How it is used could be included in > the longer function description. The longer function description would > include adding the buffer data or the buffer data hash to the IMA > measurement list and extending the PCR. > > For example, > process_buffer_measurement - measure the buffer data or the buffer data > hash > Thanks Mimi. Will update the brief and longer descriptions accordingly. > >> */ >> void process_buffer_measurement(struct inode *inode, const void *buf, int size, >> const char *eventname, enum ima_hooks func, >> - int pcr, const char *func_data) >> + int pcr, const char *func_data, >> + bool measure_buf_hash) >> { >> int ret = 0; >> const char *audit_cause = "ENOMEM"; >> @@ -807,6 +810,8 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size, >> struct ima_digest_data hdr; >> char digest[IMA_MAX_DIGEST_SIZE]; >> } hash = {}; >> + char digest_hash[IMA_MAX_DIGEST_SIZE]; >> + int hash_len = hash_digest_size[ima_hash_algo]; >> int violation = 0; >> int action = 0; >> u32 secid; >> @@ -855,6 +860,21 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size, >> goto out; >> } >> >> + if (measure_buf_hash) { >> + memcpy(digest_hash, hash.hdr.digest, hash_len); > > Instead of digest_hash and hash_len, consider naming the variables > buf_hash and buf_hashlen. > Thanks. Will do. >> + >> + ret = ima_calc_buffer_hash(digest_hash, >> + hash_len, >> + iint.ima_hash); > > There's no need for each variable to be on a separate line. > Thanks, will fix. ~Tushar
> thanks, > > Mimi > >> + if (ret < 0) { >> + audit_cause = "measure_buf_hash_error"; >> + goto out; >> + } >> + >> + event_data.buf = digest_hash; >> + event_data.buf_len = hash_len; >> + } >> + >> ret = ima_alloc_init_template(&event_data, &entry, template); >> if (ret < 0) { >> audit_cause = "alloc_entry";
| |