From ccf11dbaa07b328fa469415c362d33459c140a37 Mon Sep 17 00:00:00 2001 From: Dinghao Liu Date: Sun, 10 Jan 2021 16:02:53 +0800 Subject: evm: Fix memleak in init_desc tmp_tfm is allocated, but not freed on subsequent kmalloc failure, which leads to a memory leak. Free tmp_tfm. Fixes: d46eb3699502b ("evm: crypto hash replaced by shash") Signed-off-by: Dinghao Liu [zohar@linux.ibm.com: formatted/reworded patch description] Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_crypto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 168c3b78ac47..a6dd47eb086d 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -73,7 +73,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo) { long rc; const char *algo; - struct crypto_shash **tfm, *tmp_tfm; + struct crypto_shash **tfm, *tmp_tfm = NULL; struct shash_desc *desc; if (type == EVM_XATTR_HMAC) { @@ -118,13 +118,16 @@ unlock: alloc: desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm), GFP_KERNEL); - if (!desc) + if (!desc) { + crypto_free_shash(tmp_tfm); return ERR_PTR(-ENOMEM); + } desc->tfm = *tfm; rc = crypto_shash_init(desc); if (rc) { + crypto_free_shash(tmp_tfm); kfree(desc); return ERR_PTR(rc); } -- cgit From 2b4a2474a2027eb683bc421eff286fc617ce1d82 Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:01 -0800 Subject: IMA: generalize keyring specific measurement constructs IMA functions such as ima_match_keyring(), process_buffer_measurement(), ima_match_policy() etc. handle data specific to keyrings. Currently, these constructs are not generic to handle any func specific data. This makes it harder to extend them without code duplication. Refactor the keyring specific measurement constructs to be generic and reusable in other measurement scenarios. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 6 +++--- security/integrity/ima/ima_api.c | 6 +++--- security/integrity/ima/ima_main.c | 6 +++--- security/integrity/ima/ima_policy.c | 43 ++++++++++++++++++++++--------------- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 8e8b1e3cb847..e5622ce8cbb1 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -256,7 +256,7 @@ static inline void ima_process_queued_keys(void) {} int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, int mask, enum ima_hooks func, int *pcr, struct ima_template_desc **template_desc, - const char *keyring); + const char *func_data); int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func); int ima_collect_measurement(struct integrity_iint_cache *iint, struct file *file, void *buf, loff_t size, @@ -268,7 +268,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, struct ima_template_desc *template_desc); void process_buffer_measurement(struct inode *inode, const void *buf, int size, const char *eventname, enum ima_hooks func, - int pcr, const char *keyring); + int pcr, const char *func_data); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_alloc_init_template(struct ima_event_data *event_data, @@ -284,7 +284,7 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename); int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, enum ima_hooks func, int mask, int flags, int *pcr, struct ima_template_desc **template_desc, - const char *keyring); + const char *func_data); void ima_init_policy(void); void ima_update_policy(void); void ima_update_policy_flag(void); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 4f39fb93f278..e76499b1ce78 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -170,7 +170,7 @@ err_out: * @func: caller identifier * @pcr: pointer filled in if matched measure policy sets pcr= * @template_desc: pointer filled in if matched measure policy sets template= - * @keyring: keyring name used to determine the action + * @func_data: func specific data, may be NULL * * The policy is defined in terms of keypairs: * subj=, obj=, type=, func=, mask=, fsmagic= @@ -186,14 +186,14 @@ err_out: int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, int mask, enum ima_hooks func, int *pcr, struct ima_template_desc **template_desc, - const char *keyring) + const char *func_data) { int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH; flags &= ima_policy_flag; return ima_match_policy(inode, cred, secid, func, mask, flags, pcr, - template_desc, keyring); + template_desc, func_data); } /* diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f87cb29329e9..0c645699c7fb 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -816,13 +816,13 @@ int ima_post_load_data(char *buf, loff_t size, * @eventname: event name to be used for the buffer entry. * @func: IMA hook * @pcr: pcr to extend the measurement - * @keyring: keyring name to determine the action to be performed + * @func_data: func specific data, may be NULL * * Based on policy, the buffer is measured into the ima log. */ void process_buffer_measurement(struct inode *inode, const void *buf, int size, const char *eventname, enum ima_hooks func, - int pcr, const char *keyring) + int pcr, const char *func_data) { int ret = 0; const char *audit_cause = "ENOMEM"; @@ -861,7 +861,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size, if (func) { security_task_getsecid(current, &secid); action = ima_get_action(inode, current_cred(), secid, 0, func, - &pcr, &template, keyring); + &pcr, &template, func_data); if (!(action & IMA_MEASURE)) return; } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 823a0c1379cb..b93966034368 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -453,30 +453,40 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, } /** - * ima_match_keyring - determine whether the keyring matches the measure rule + * ima_match_rule_data - determine whether func_data matches the policy rule * @rule: a pointer to a rule - * @keyring: name of the keyring to match against the measure rule + * @func_data: data to match against the measure rule data * @cred: a pointer to a credentials structure for user validation * - * Returns true if keyring matches one in the rule, false otherwise. + * Returns true if func_data matches one in the rule, false otherwise. */ -static bool ima_match_keyring(struct ima_rule_entry *rule, - const char *keyring, const struct cred *cred) +static bool ima_match_rule_data(struct ima_rule_entry *rule, + const char *func_data, + const struct cred *cred) { + const struct ima_rule_opt_list *opt_list = NULL; bool matched = false; size_t i; if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid)) return false; - if (!rule->keyrings) - return true; + switch (rule->func) { + case KEY_CHECK: + if (!rule->keyrings) + return true; + + opt_list = rule->keyrings; + break; + default: + return false; + } - if (!keyring) + if (!func_data) return false; - for (i = 0; i < rule->keyrings->count; i++) { - if (!strcmp(rule->keyrings->items[i], keyring)) { + for (i = 0; i < opt_list->count; i++) { + if (!strcmp(opt_list->items[i], func_data)) { matched = true; break; } @@ -493,20 +503,20 @@ static bool ima_match_keyring(struct ima_rule_entry *rule, * @secid: the secid of the task to be validated * @func: LIM hook identifier * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) - * @keyring: keyring name to check in policy for KEY_CHECK func + * @func_data: func specific data, may be NULL * * Returns true on rule match, false on failure. */ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, const struct cred *cred, u32 secid, enum ima_hooks func, int mask, - const char *keyring) + const char *func_data) { int i; if (func == KEY_CHECK) { return (rule->flags & IMA_FUNC) && (rule->func == func) && - ima_match_keyring(rule, keyring, cred); + ima_match_rule_data(rule, func_data, cred); } if ((rule->flags & IMA_FUNC) && (rule->func != func && func != POST_SETATTR)) @@ -610,8 +620,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) * @pcr: set the pcr to extend * @template_desc: the template that should be used for this rule - * @keyring: the keyring name, if given, to be used to check in the policy. - * keyring can be NULL if func is anything other than KEY_CHECK. + * @func_data: func specific data, may be NULL * * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type) * conditions. @@ -623,7 +632,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, enum ima_hooks func, int mask, int flags, int *pcr, struct ima_template_desc **template_desc, - const char *keyring) + const char *func_data) { struct ima_rule_entry *entry; int action = 0, actmask = flags | (flags << 1); @@ -638,7 +647,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, continue; if (!ima_match_rules(entry, inode, cred, secid, func, mask, - keyring)) + func_data)) continue; action |= entry->flags & IMA_ACTION_FLAGS; -- cgit From 291af651b350817f7f1cbe308faaf7fa7af2a92c Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:02 -0800 Subject: IMA: add support to measure buffer data hash The original IMA buffer data measurement sizes were small (e.g. boot command line), but the new buffer data measurement use cases have data sizes that are a lot larger. Just as IMA measures the file data hash, not the file data, IMA should similarly support the option for measuring buffer data hash. Introduce a boolean parameter to support measuring buffer data hash, which would be much smaller, instead of the buffer itself. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 3 ++- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_asymmetric_keys.c | 2 +- security/integrity/ima/ima_main.c | 29 +++++++++++++++++++++++----- security/integrity/ima/ima_queue_keys.c | 3 ++- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e5622ce8cbb1..0b4634515839 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -268,7 +268,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, struct ima_template_desc *template_desc); 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 buf_hash); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_alloc_init_template(struct ima_event_data *event_data, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 8361941ee0a1..46ffa38bab12 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -352,7 +352,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint, if ((rc == -EPERM) && (iint->flags & IMA_MEASURE)) process_buffer_measurement(NULL, digest, digestsize, "blacklisted-hash", NONE, - pcr, NULL); + pcr, NULL, false); } return rc; diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c index 1c68c500c26f..a74095793936 100644 --- a/security/integrity/ima/ima_asymmetric_keys.c +++ b/security/integrity/ima/ima_asymmetric_keys.c @@ -60,5 +60,5 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, */ process_buffer_measurement(NULL, payload, payload_len, keyring->description, KEY_CHECK, 0, - keyring->description); + keyring->description, false); } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 0c645699c7fb..250e52114230 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -809,7 +809,7 @@ int ima_post_load_data(char *buf, loff_t size, } /* - * process_buffer_measurement - Measure the buffer to ima log. + * process_buffer_measurement - Measure the buffer or the buffer data hash * @inode: inode associated with the object being measured (NULL for KEY_CHECK) * @buf: pointer to the buffer that needs to be added to the log. * @size: size of buffer(in bytes). @@ -817,12 +817,14 @@ int ima_post_load_data(char *buf, loff_t size, * @func: IMA hook * @pcr: pcr to extend the measurement * @func_data: func specific data, may be NULL + * @buf_hash: measure buffer data hash * - * Based on policy, the buffer is measured into the ima log. + * Based on policy, either the buffer data or buffer data hash is measured */ 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 buf_hash) { int ret = 0; const char *audit_cause = "ENOMEM"; @@ -837,6 +839,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 digest_hash_len = hash_digest_size[ima_hash_algo]; int violation = 0; int action = 0; u32 secid; @@ -879,13 +883,27 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size, goto out; } + if (buf_hash) { + memcpy(digest_hash, hash.hdr.digest, digest_hash_len); + + ret = ima_calc_buffer_hash(digest_hash, digest_hash_len, + iint.ima_hash); + if (ret < 0) { + audit_cause = "hashing_error"; + goto out; + } + + event_data.buf = digest_hash; + event_data.buf_len = digest_hash_len; + } + ret = ima_alloc_init_template(&event_data, &entry, template); if (ret < 0) { audit_cause = "alloc_entry"; goto out; } - ret = ima_store_template(entry, violation, NULL, buf, pcr); + ret = ima_store_template(entry, violation, NULL, event_data.buf, pcr); if (ret < 0) { audit_cause = "store_entry"; ima_free_template_entry(entry); @@ -920,7 +938,8 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) return; process_buffer_measurement(file_inode(f.file), buf, size, - "kexec-cmdline", KEXEC_CMDLINE, 0, NULL); + "kexec-cmdline", KEXEC_CMDLINE, 0, NULL, + false); fdput(f); } diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c index 69a8626a35c0..c2f2ad34f9b7 100644 --- a/security/integrity/ima/ima_queue_keys.c +++ b/security/integrity/ima/ima_queue_keys.c @@ -162,7 +162,8 @@ void ima_process_queued_keys(void) entry->payload_len, entry->keyring_name, KEY_CHECK, 0, - entry->keyring_name); + entry->keyring_name, + false); list_del(&entry->list); ima_free_key_entry(entry); } -- cgit From d6e645012d97164609260ac567b304681734c5e2 Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:03 -0800 Subject: IMA: define a hook to measure kernel integrity critical data IMA provides capabilities to measure file and buffer data. However, various data structures, policies, and states stored in kernel memory also impact the integrity of the system. Several kernel subsystems contain such integrity critical data. These kernel subsystems help protect the integrity of the system. Currently, IMA does not provide a generic function for measuring kernel integrity critical data. Define ima_measure_critical_data, a new IMA hook, to measure kernel integrity critical data. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- include/linux/ima.h | 7 +++++++ security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_main.c | 24 ++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index 7db9cca1af34..59bd90ac3c35 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -31,6 +31,9 @@ extern void ima_post_path_mknod(struct dentry *dentry); extern int ima_file_hash(struct file *file, char *buf, size_t buf_size); extern int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size); extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size); +extern void ima_measure_critical_data(const char *event_name, + const void *buf, size_t buf_len, + bool hash); #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM extern void ima_appraise_parse_cmdline(void); @@ -128,6 +131,10 @@ static inline int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size } static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {} + +static inline void ima_measure_critical_data(const char *event_name, + const void *buf, size_t buf_len, + bool hash) {} #endif /* CONFIG_IMA */ #ifndef CONFIG_IMA_KEXEC diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 0b4634515839..aa312472c7c5 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -201,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest) hook(POLICY_CHECK, policy) \ hook(KEXEC_CMDLINE, kexec_cmdline) \ hook(KEY_CHECK, key) \ + hook(CRITICAL_DATA, critical_data) \ hook(MAX_CHECK, none) #define __ima_hook_enumify(ENUM, str) ENUM, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index e76499b1ce78..1dd70dc68ffd 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -176,7 +176,7 @@ err_out: * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK - * | KEXEC_CMDLINE | KEY_CHECK + * | KEXEC_CMDLINE | KEY_CHECK | CRITICAL_DATA * mask: contains the permission mask * fsmagic: hex value * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 250e52114230..251e7b4006f4 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -943,6 +943,30 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) fdput(f); } +/** + * ima_measure_critical_data - measure kernel integrity critical data + * @event_name: event name for the record in the IMA measurement list + * @buf: pointer to buffer data + * @buf_len: length of buffer data (in bytes) + * @hash: measure buffer data hash + * + * Measure data critical to the integrity of the kernel into the IMA log + * and extend the pcr. Examples of critical data could be various data + * structures, policies, and states stored in kernel memory that can + * impact the integrity of the system. + */ +void ima_measure_critical_data(const char *event_name, + const void *buf, size_t buf_len, + bool hash) +{ + if (!event_name || !buf || !buf_len) + return; + + process_buffer_measurement(NULL, buf, buf_len, event_name, + CRITICAL_DATA, 0, NULL, + hash); +} + static int __init init_ima(void) { int error; -- cgit From c4e43aa2eeb0cffcf0b17e0a60a9d212de9c49df Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:04 -0800 Subject: IMA: add policy rule to measure critical data A new IMA policy rule is needed for the IMA hook ima_measure_critical_data() and the corresponding func CRITICAL_DATA for measuring the input buffer. The policy rule should ensure the buffer would get measured only when the policy rule allows the action. The policy rule should also support the necessary constraints (flags etc.) for integrity critical buffer data measurements. Add policy rule support for measuring integrity critical data. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 2 +- security/integrity/ima/ima_policy.c | 29 +++++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index e35263f97fc1..6ec7daa87cba 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -32,7 +32,7 @@ Description: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK]MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] - [KEXEC_CMDLINE] [KEY_CHECK] + [KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index b93966034368..96ba4273c4d0 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -478,6 +478,8 @@ static bool ima_match_rule_data(struct ima_rule_entry *rule, opt_list = rule->keyrings; break; + case CRITICAL_DATA: + return true; default: return false; } @@ -514,13 +516,19 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, { int i; - if (func == KEY_CHECK) { - return (rule->flags & IMA_FUNC) && (rule->func == func) && - ima_match_rule_data(rule, func_data, cred); - } if ((rule->flags & IMA_FUNC) && (rule->func != func && func != POST_SETATTR)) return false; + + switch (func) { + case KEY_CHECK: + case CRITICAL_DATA: + return ((rule->func == func) && + ima_match_rule_data(rule, func_data, cred)); + default: + break; + } + if ((rule->flags & IMA_MASK) && (rule->mask != mask && func != POST_SETATTR)) return false; @@ -1115,6 +1123,17 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) if (ima_rule_contains_lsm_cond(entry)) return false; + break; + case CRITICAL_DATA: + if (entry->action & ~(MEASURE | DONT_MEASURE)) + return false; + + if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR)) + return false; + + if (ima_rule_contains_lsm_cond(entry)) + return false; + break; default: return false; @@ -1247,6 +1266,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else if (IS_ENABLED(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) && strcmp(args[0].from, "KEY_CHECK") == 0) entry->func = KEY_CHECK; + else if (strcmp(args[0].from, "CRITICAL_DATA") == 0) + entry->func = CRITICAL_DATA; else result = -EINVAL; if (!result) -- cgit From 47d76a4840501c1cefb3fbce777a86c58b02532b Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:05 -0800 Subject: IMA: limit critical data measurement based on a label Integrity critical data may belong to a single subsystem or it may arise from cross subsystem interaction. Currently there is no mechanism to group or limit the data based on certain label. Limiting and grouping critical data based on a label would make it flexible and configurable to measure. Define "label:=", a new IMA policy condition, for the IMA func CRITICAL_DATA to allow grouping and limiting measurement of integrity critical data. Limit the measurement to the labels that are specified in the IMA policy - CRITICAL_DATA+"label:=". If "label:=" is not provided with the func CRITICAL_DATA, measure all the input integrity critical data. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 2 ++ security/integrity/ima/ima_policy.c | 37 +++++++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 6ec7daa87cba..54fe1c15ed50 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -52,6 +52,8 @@ Description: template:= name of a defined IMA template type (eg, ima-ng). Only valid when action is "measure". pcr:= decimal value + label:= [data_label] + data_label:= a unique string used for grouping and limiting critical data. default policy: # PROC_SUPER_MAGIC diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 96ba4273c4d0..2c9db2d0b434 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -34,6 +34,7 @@ #define IMA_PCR 0x0100 #define IMA_FSNAME 0x0200 #define IMA_KEYRINGS 0x0400 +#define IMA_LABEL 0x0800 #define UNKNOWN 0 #define MEASURE 0x0001 /* same as IMA_MEASURE */ @@ -85,6 +86,7 @@ struct ima_rule_entry { } lsm[MAX_LSM_RULES]; char *fsname; struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ + struct ima_rule_opt_list *label; /* Measure data grouped under this label */ struct ima_template_desc *template; }; @@ -479,7 +481,11 @@ static bool ima_match_rule_data(struct ima_rule_entry *rule, opt_list = rule->keyrings; break; case CRITICAL_DATA: - return true; + if (!rule->label) + return true; + + opt_list = rule->label; + break; default: return false; } @@ -924,7 +930,7 @@ enum { Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, Opt_appraise_type, Opt_appraise_flag, Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings, - Opt_err + Opt_label, Opt_err }; static const match_table_t policy_tokens = { @@ -961,6 +967,7 @@ static const match_table_t policy_tokens = { {Opt_pcr, "pcr=%s"}, {Opt_template, "template=%s"}, {Opt_keyrings, "keyrings=%s"}, + {Opt_label, "label=%s"}, {Opt_err, NULL} }; @@ -1128,7 +1135,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) if (entry->action & ~(MEASURE | DONT_MEASURE)) return false; - if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR)) + if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR | + IMA_LABEL)) return false; if (ima_rule_contains_lsm_cond(entry)) @@ -1338,6 +1346,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->flags |= IMA_KEYRINGS; break; + case Opt_label: + ima_log_string(ab, "label", args[0].from); + + if (entry->label) { + result = -EINVAL; + break; + } + + entry->label = ima_alloc_rule_opt_list(args); + if (IS_ERR(entry->label)) { + result = PTR_ERR(entry->label); + entry->label = NULL; + break; + } + + entry->flags |= IMA_LABEL; + break; case Opt_fsuuid: ima_log_string(ab, "fsuuid", args[0].from); @@ -1718,6 +1743,12 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, " "); } + if (entry->flags & IMA_LABEL) { + seq_puts(m, "label="); + ima_show_rule_opt_list(m, entry->label); + seq_puts(m, " "); + } + if (entry->flags & IMA_PCR) { snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr); seq_printf(m, pt(Opt_pcr), tbuf); -- cgit From 9f5d7d23cc5ec61a92076b73665fcb9aaa5bb5a0 Mon Sep 17 00:00:00 2001 From: Tushar Sugandhi Date: Thu, 7 Jan 2021 20:07:06 -0800 Subject: IMA: extend critical data hook to limit the measurement based on a label The IMA hook ima_measure_critical_data() does not support a way to specify the source of the critical data provider. Thus, the data measurement cannot be constrained based on the data source label in the IMA policy. Extend the IMA hook ima_measure_critical_data() to support passing the data source label as an input parameter, so that the policy rule can be used to limit the measurements based on the label. Signed-off-by: Tushar Sugandhi Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- include/linux/ima.h | 7 +++++-- security/integrity/ima/ima_main.c | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index 59bd90ac3c35..2ac834badbbe 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -31,7 +31,8 @@ extern void ima_post_path_mknod(struct dentry *dentry); extern int ima_file_hash(struct file *file, char *buf, size_t buf_size); extern int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size); extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size); -extern void ima_measure_critical_data(const char *event_name, +extern void ima_measure_critical_data(const char *event_label, + const char *event_name, const void *buf, size_t buf_len, bool hash); @@ -132,9 +133,11 @@ static inline int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {} -static inline void ima_measure_critical_data(const char *event_name, +static inline void ima_measure_critical_data(const char *event_label, + const char *event_name, const void *buf, size_t buf_len, bool hash) {} + #endif /* CONFIG_IMA */ #ifndef CONFIG_IMA_KEXEC diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 251e7b4006f4..6a429846f90a 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -945,6 +945,7 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) /** * ima_measure_critical_data - measure kernel integrity critical data + * @event_label: unique event label for grouping and limiting critical data * @event_name: event name for the record in the IMA measurement list * @buf: pointer to buffer data * @buf_len: length of buffer data (in bytes) @@ -955,15 +956,16 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) * structures, policies, and states stored in kernel memory that can * impact the integrity of the system. */ -void ima_measure_critical_data(const char *event_name, +void ima_measure_critical_data(const char *event_label, + const char *event_name, const void *buf, size_t buf_len, bool hash) { - if (!event_name || !buf || !buf_len) + if (!event_name || !event_label || !buf || !buf_len) return; process_buffer_measurement(NULL, buf, buf_len, event_name, - CRITICAL_DATA, 0, NULL, + CRITICAL_DATA, 0, event_label, hash); } -- cgit From 03cee168366621db85000cec47f5cefdb83e049b Mon Sep 17 00:00:00 2001 From: Lakshmi Ramasubramanian Date: Thu, 7 Jan 2021 20:07:07 -0800 Subject: IMA: define a builtin critical data measurement policy Define a new critical data builtin policy to allow measuring early kernel integrity critical data before a custom IMA policy is loaded. Update the documentation on kernel parameters to document the new critical data builtin policy. Signed-off-by: Lakshmi Ramasubramanian Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- Documentation/admin-guide/kernel-parameters.txt | 5 ++++- security/integrity/ima/ima_policy.c | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 9e3cdb271d06..65a0c4c9ab18 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1746,7 +1746,7 @@ ima_policy= [IMA] The builtin policies to load during IMA setup. Format: "tcb | appraise_tcb | secure_boot | - fail_securely" + fail_securely | critical_data" The "tcb" policy measures all programs exec'd, files mmap'd for exec, and all files opened with the read @@ -1765,6 +1765,9 @@ filesystems with the SB_I_UNVERIFIABLE_SIGNATURE flag. + The "critical_data" policy measures kernel integrity + critical data. + ima_tcb [IMA] Deprecated. Use ima_policy= instead. Load a policy which meets the needs of the Trusted Computing Base. This means IMA will measure all diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 2c9db2d0b434..9b45d064a87d 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -206,6 +206,10 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = { .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, }; +static struct ima_rule_entry critical_data_rules[] __ro_after_init = { + {.action = MEASURE, .func = CRITICAL_DATA, .flags = IMA_FUNC}, +}; + /* An array of architecture specific rules */ static struct ima_rule_entry *arch_policy_entry __ro_after_init; @@ -228,6 +232,7 @@ __setup("ima_tcb", default_measure_policy_setup); static bool ima_use_appraise_tcb __initdata; static bool ima_use_secure_boot __initdata; +static bool ima_use_critical_data __initdata; static bool ima_fail_unverifiable_sigs __ro_after_init; static int __init policy_setup(char *str) { @@ -242,6 +247,8 @@ static int __init policy_setup(char *str) ima_use_appraise_tcb = true; else if (strcmp(p, "secure_boot") == 0) ima_use_secure_boot = true; + else if (strcmp(p, "critical_data") == 0) + ima_use_critical_data = true; else if (strcmp(p, "fail_securely") == 0) ima_fail_unverifiable_sigs = true; else @@ -871,6 +878,11 @@ void __init ima_init_policy(void) ARRAY_SIZE(default_appraise_rules), IMA_DEFAULT_POLICY); + if (ima_use_critical_data) + add_rules(critical_data_rules, + ARRAY_SIZE(critical_data_rules), + IMA_DEFAULT_POLICY); + ima_update_policy_flag(); } -- cgit From fdd1ffe8a812b1109388e4bc389e57b2695ad095 Mon Sep 17 00:00:00 2001 From: Lakshmi Ramasubramanian Date: Thu, 14 Jan 2021 11:15:22 -0800 Subject: selinux: include a consumer of the new IMA critical data hook SELinux stores the active policy in memory, so the changes to this data at runtime would have an impact on the security guarantees provided by SELinux. Measuring in-memory SELinux policy through IMA subsystem provides a secure way for the attestation service to remotely validate the policy contents at runtime. Measure the hash of the loaded policy by calling the IMA hook ima_measure_critical_data(). Since the size of the loaded policy can be large (several MB), measure the hash of the policy instead of the entire policy to avoid bloating the IMA log entry. To enable SELinux data measurement, the following steps are required: 1, Add "ima_policy=critical_data" to the kernel command line arguments to enable measuring SELinux data at boot time. For example, BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data 2, Add the following rule to /etc/ima/ima-policy measure func=CRITICAL_DATA label=selinux Sample measurement of the hash of SELinux policy: To verify the measured data with the current SELinux policy run the following commands and verify the output hash values match. sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1 grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6 Note that the actual verification of SELinux policy would require loading the expected policy into an identical kernel on a pristine/known-safe system and run the sha256sum /sys/kernel/selinux/policy there to get the expected hash. Signed-off-by: Lakshmi Ramasubramanian Suggested-by: Stephen Smalley Acked-by: Paul Moore Reviewed-by: Tyler Hicks Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 3 +- security/selinux/Makefile | 2 ++ security/selinux/ima.c | 44 +++++++++++++++++++++++++ security/selinux/include/ima.h | 24 ++++++++++++++ security/selinux/include/security.h | 3 +- security/selinux/ss/services.c | 64 +++++++++++++++++++++++++++++++----- 6 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 security/selinux/ima.c create mode 100644 security/selinux/include/ima.h diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 54fe1c15ed50..8365596cb42b 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -52,8 +52,9 @@ Description: template:= name of a defined IMA template type (eg, ima-ng). Only valid when action is "measure". pcr:= decimal value - label:= [data_label] + label:= [selinux]|[data_label] data_label:= a unique string used for grouping and limiting critical data. + For example, "selinux" to measure critical data for SELinux. default policy: # PROC_SUPER_MAGIC diff --git a/security/selinux/Makefile b/security/selinux/Makefile index 4d8e0e8adf0b..776162444882 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o +selinux-$(CONFIG_IMA) += ima.o + ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h diff --git a/security/selinux/ima.c b/security/selinux/ima.c new file mode 100644 index 000000000000..03715893ff97 --- /dev/null +++ b/security/selinux/ima.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Microsoft Corporation + * + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) + * + * Measure critical data structures maintainted by SELinux + * using IMA subsystem. + */ +#include +#include +#include "security.h" +#include "ima.h" + +/* + * selinux_ima_measure_state - Measure hash of the SELinux policy + * + * @state: selinux state struct + * + * NOTE: This function must be called with policy_mutex held. + */ +void selinux_ima_measure_state(struct selinux_state *state) +{ + void *policy = NULL; + size_t policy_len; + int rc = 0; + + /* + * Measure SELinux policy only after initialization is completed. + */ + if (!selinux_initialized(state)) + return; + + rc = security_read_state_kernel(state, &policy, &policy_len); + if (rc) { + pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc); + return; + } + + ima_measure_critical_data("selinux", "selinux-policy-hash", + policy, policy_len, true); + + vfree(policy); +} diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h new file mode 100644 index 000000000000..d69c36611423 --- /dev/null +++ b/security/selinux/include/ima.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2021 Microsoft Corporation + * + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) + * + * Measure critical data structures maintainted by SELinux + * using IMA subsystem. + */ + +#ifndef _SELINUX_IMA_H_ +#define _SELINUX_IMA_H_ + +#include "security.h" + +#ifdef CONFIG_IMA +extern void selinux_ima_measure_state(struct selinux_state *selinux_state); +#else +static inline void selinux_ima_measure_state(struct selinux_state *selinux_state) +{ +} +#endif + +#endif /* _SELINUX_IMA_H_ */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 3cc8bab31ea8..29cae32d3fc5 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state, struct selinux_policy *policy); int security_read_policy(struct selinux_state *state, void **data, size_t *len); - +int security_read_state_kernel(struct selinux_state *state, + void **data, size_t *len); int security_policycap_supported(struct selinux_state *state, unsigned int req_cap); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 597b79703584..2106b5d383e7 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -65,6 +65,7 @@ #include "ebitmap.h" #include "audit.h" #include "policycap_names.h" +#include "ima.h" /* Forward declaration. */ static int context_struct_to_string(struct policydb *policydb, @@ -2178,6 +2179,7 @@ static void selinux_notify_policy_change(struct selinux_state *state, selinux_status_update_policyload(state, seqno); selinux_netlbl_cache_invalidate(); selinux_xfrm_notify_policyload(); + selinux_ima_measure_state(state); } void selinux_policy_commit(struct selinux_state *state, @@ -3873,8 +3875,33 @@ out: } #endif /* CONFIG_NETLABEL */ +/** + * __security_read_policy - read the policy. + * @policy: SELinux policy + * @data: binary policy data + * @len: length of data in bytes + * + */ +static int __security_read_policy(struct selinux_policy *policy, + void *data, size_t *len) +{ + int rc; + struct policy_file fp; + + fp.data = data; + fp.len = *len; + + rc = policydb_write(&policy->policydb, &fp); + if (rc) + return rc; + + *len = (unsigned long)fp.data - (unsigned long)data; + return 0; +} + /** * security_read_policy - read the policy. + * @state: selinux_state * @data: binary policy data * @len: length of data in bytes * @@ -3883,8 +3910,6 @@ int security_read_policy(struct selinux_state *state, void **data, size_t *len) { struct selinux_policy *policy; - int rc; - struct policy_file fp; policy = rcu_dereference_protected( state->policy, lockdep_is_held(&state->policy_mutex)); @@ -3896,14 +3921,35 @@ int security_read_policy(struct selinux_state *state, if (!*data) return -ENOMEM; - fp.data = *data; - fp.len = *len; + return __security_read_policy(policy, *data, len); +} - rc = policydb_write(&policy->policydb, &fp); - if (rc) - return rc; +/** + * security_read_state_kernel - read the policy. + * @state: selinux_state + * @data: binary policy data + * @len: length of data in bytes + * + * Allocates kernel memory for reading SELinux policy. + * This function is for internal use only and should not + * be used for returning data to user space. + * + * This function must be called with policy_mutex held. + */ +int security_read_state_kernel(struct selinux_state *state, + void **data, size_t *len) +{ + struct selinux_policy *policy; - *len = (unsigned long)fp.data - (unsigned long)*data; - return 0; + policy = rcu_dereference_protected( + state->policy, lockdep_is_held(&state->policy_mutex)); + if (!policy) + return -EINVAL; + + *len = policy->policydb.len; + *data = vmalloc(*len); + if (!*data) + return -ENOMEM; + return __security_read_policy(policy, *data, len); } -- cgit From b3f82afc1041a6a7d5347a01883f4aab7ec133b2 Mon Sep 17 00:00:00 2001 From: Raphael Gianotti Date: Tue, 26 Jan 2021 11:14:53 -0800 Subject: IMA: Measure kernel version in early boot The integrity of a kernel can be verified by the boot loader on cold boot, and during kexec, by the current running kernel, before it is loaded. However, it is still possible that the new kernel being loaded is older than the current kernel, and/or has known vulnerabilities. Therefore, it is imperative that an attestation service be able to verify the version of the kernel being loaded on the client, from cold boot and subsequent kexec system calls, ensuring that only kernels with versions known to be good are loaded. Measure the kernel version using ima_measure_critical_data() early on in the boot sequence, reducing the chances of known kernel vulnerabilities being exploited. With IMA being part of the kernel, this overall approach makes the measurement itself more trustworthy. To enable measuring the kernel version "ima_policy=critical_data" needs to be added to the kernel command line arguments. For example, BOOT_IMAGE=/boot/vmlinuz-5.11.0-rc3+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset ima_policy=critical_data If runtime measurement of the kernel version is ever needed, the following should be added to /etc/ima/ima-policy: measure func=CRITICAL_DATA label=kernel_info To extract the measured data after boot, the following command can be used: grep -m 1 "kernel_version" \ /sys/kernel/security/integrity/ima/ascii_runtime_measurements Sample output from the command above: 10 a8297d408e9d5155728b619761d0dd4cedf5ef5f ima-buf sha256:5660e19945be0119bc19cbbf8d9c33a09935ab5d30dad48aa11f879c67d70988 kernel_version 352e31312e302d7263332d31363138372d676564623634666537383234342d6469727479 The above hex-ascii string corresponds to the kernel version (e.g. xxd -r -p): 5.11.0-rc3-16187-gedb64fe78244-dirty Signed-off-by: Raphael Gianotti Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 2 +- security/integrity/ima/ima_init.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 8365596cb42b..bc8e1cbe5e61 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -52,7 +52,7 @@ Description: template:= name of a defined IMA template type (eg, ima-ng). Only valid when action is "measure". pcr:= decimal value - label:= [selinux]|[data_label] + label:= [selinux]|[kernel_info]|[data_label] data_label:= a unique string used for grouping and limiting critical data. For example, "selinux" to measure critical data for SELinux. diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 4902fe7bd570..6e8742916d1d 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include "ima.h" @@ -147,5 +149,8 @@ int __init ima_init(void) ima_init_key_queue(); + ima_measure_critical_data("kernel_info", "kernel_version", + UTS_RELEASE, strlen(UTS_RELEASE), false); + return rc; } -- cgit From 6d14c6517885fa68524238787420511b87d671df Mon Sep 17 00:00:00 2001 From: Lakshmi Ramasubramanian Date: Thu, 4 Feb 2021 09:49:50 -0800 Subject: ima: Free IMA measurement buffer on error IMA allocates kernel virtual memory to carry forward the measurement list, from the current kernel to the next kernel on kexec system call, in ima_add_kexec_buffer() function. In error code paths this memory is not freed resulting in memory leak. Free the memory allocated for the IMA measurement list in the error code paths in ima_add_kexec_buffer() function. Signed-off-by: Lakshmi Ramasubramanian Suggested-by: Tyler Hicks Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_kexec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 121de3e04af2..206ddcaa5c67 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -119,6 +119,7 @@ void ima_add_kexec_buffer(struct kimage *image) ret = kexec_add_buffer(&kbuf); if (ret) { pr_err("Error passing over kexec measurement buffer.\n"); + vfree(kexec_buffer); return; } -- cgit From f31e3386a4e92ba6eda7328cb508462956c94c64 Mon Sep 17 00:00:00 2001 From: Lakshmi Ramasubramanian Date: Thu, 4 Feb 2021 09:49:51 -0800 Subject: ima: Free IMA measurement buffer after kexec syscall IMA allocates kernel virtual memory to carry forward the measurement list, from the current kernel to the next kernel on kexec system call, in ima_add_kexec_buffer() function. This buffer is not freed before completing the kexec system call resulting in memory leak. Add ima_buffer field in "struct kimage" to store the virtual address of the buffer allocated for the IMA measurement list. Free the memory allocated for the IMA measurement list in kimage_file_post_load_cleanup() function. Signed-off-by: Lakshmi Ramasubramanian Suggested-by: Tyler Hicks Reviewed-by: Thiago Jung Bauermann Reviewed-by: Tyler Hicks Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") Signed-off-by: Mimi Zohar --- include/linux/kexec.h | 5 +++++ kernel/kexec_file.c | 5 +++++ security/integrity/ima/ima_kexec.c | 2 ++ 3 files changed, 12 insertions(+) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 9e93bef52968..5f61389f5f36 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -300,6 +300,11 @@ struct kimage { /* Information for loading purgatory */ struct purgatory_info purgatory_info; #endif + +#ifdef CONFIG_IMA_KEXEC + /* Virtual address of IMA measurement buffer for kexec syscall */ + void *ima_buffer; +#endif }; /* kexec interface functions */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index b02086d70492..5c3447cf7ad5 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image) vfree(pi->sechdrs); pi->sechdrs = NULL; +#ifdef CONFIG_IMA_KEXEC + vfree(image->ima_buffer); + image->ima_buffer = NULL; +#endif /* CONFIG_IMA_KEXEC */ + /* See if architecture has anything to cleanup post load */ arch_kimage_file_post_load_cleanup(image); diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 206ddcaa5c67..e29bea3dd4cc 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -129,6 +129,8 @@ void ima_add_kexec_buffer(struct kimage *image) return; } + image->ima_buffer = kexec_buffer; + pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n", kbuf.mem); } -- cgit From f6692213b5045dc461ce0858fb18cf46f328c202 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 10 Feb 2021 08:01:31 +0000 Subject: integrity: Make function integrity_add_key() static The sparse tool complains as follows: security/integrity/digsig.c:146:12: warning: symbol 'integrity_add_key' was not declared. Should it be static? This function is not used outside of digsig.c, so this commit marks it static. Reported-by: Hulk Robot Fixes: 60740accf784 ("integrity: Load certs to the platform keyring") Signed-off-by: Wei Yongjun Reviewed-by: Kees Cook Reviewed-by: Nayna Jain Signed-off-by: Mimi Zohar --- security/integrity/digsig.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 0f518dcfde05..250fb0836156 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -143,8 +143,8 @@ out: return __integrity_init_keyring(id, perm, restriction); } -int __init integrity_add_key(const unsigned int id, const void *data, - off_t size, key_perm_t perm) +static int __init integrity_add_key(const unsigned int id, const void *data, + off_t size, key_perm_t perm) { key_ref_t key; int rc = 0; -- cgit