From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> To: zohar@linux.ibm.com, James.Bottomley@HansenPartnership.com, linux-integrity@vger.kernel.org Cc: dhowells@redhat.com, sashal@kernel.org, linux-kernel@vger.kernel.org, keyrings@vger.kernel.org, linux-crypto@vger.kernel.org Subject: [PATCH v1] IMA: pre-allocate buffer to hold keyrings string Date: Thu, 16 Jan 2020 15:46:23 -0800 [thread overview] Message-ID: <20200116234623.2959-1-nramas@linux.microsoft.com> (raw) ima_match_keyring() is called while holding rcu read lock. Since this function executes in atmomic context, it should not call any function that can sleep (such as kstrdup()). This patch pre-allocates a buffer to hold the keyrings string read from the IMA policy and uses that to match the given keyring. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Fixes: e9085e0ad38a ("IMA: Add support to limit measuring keys") --- security/integrity/ima/ima_policy.c | 50 ++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 9963863d6c92..180e2069e075 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -208,6 +208,10 @@ static LIST_HEAD(ima_policy_rules); static LIST_HEAD(ima_temp_rules); static struct list_head *ima_rules; +/* Pre-allocated buffer used for matching keyrings. */ +static char *ima_keyrings; +static size_t ima_keyrings_len; + static int ima_policy __initdata; static int __init default_measure_policy_setup(char *str) @@ -369,7 +373,7 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, static bool ima_match_keyring(struct ima_rule_entry *rule, const char *keyring, const struct cred *cred) { - char *keyrings, *next_keyring, *keyrings_ptr; + char *next_keyring, *keyrings_ptr; bool matched = false; if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid)) @@ -381,15 +385,13 @@ static bool ima_match_keyring(struct ima_rule_entry *rule, if (!keyring) return false; - keyrings = kstrdup(rule->keyrings, GFP_KERNEL); - if (!keyrings) - return false; + strcpy(ima_keyrings, rule->keyrings); /* * "keyrings=" is specified in the policy in the format below: * keyrings=.builtin_trusted_keys|.ima|.evm */ - keyrings_ptr = keyrings; + keyrings_ptr = ima_keyrings; while ((next_keyring = strsep(&keyrings_ptr, "|")) != NULL) { if (!strcmp(next_keyring, keyring)) { matched = true; @@ -397,8 +399,6 @@ static bool ima_match_keyring(struct ima_rule_entry *rule, } } - kfree(keyrings); - return matched; } @@ -949,6 +949,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) bool uid_token; struct ima_template_desc *template_desc; int result = 0; + size_t keyrings_len; ab = integrity_audit_log_start(audit_context(), GFP_KERNEL, AUDIT_INTEGRITY_POLICY_RULE); @@ -1114,14 +1115,47 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) case Opt_keyrings: ima_log_string(ab, "keyrings", args[0].from); + keyrings_len = strlen(args[0].from) + 1; + if ((entry->keyrings) || (entry->action != MEASURE) || - (entry->func != KEY_CHECK)) { + (entry->func != KEY_CHECK) || + (keyrings_len < 2)) { result = -EINVAL; break; } + + if (ima_keyrings) { + if (keyrings_len > ima_keyrings_len) { + char *tmpbuf; + + tmpbuf = krealloc(ima_keyrings, + keyrings_len, + GFP_KERNEL); + if (!tmpbuf) { + result = -ENOMEM; + break; + } + + ima_keyrings = tmpbuf; + ima_keyrings_len = keyrings_len; + } + } else { + ima_keyrings = kzalloc(keyrings_len, + GFP_KERNEL); + if (!ima_keyrings) { + result = -ENOMEM; + break; + } + + ima_keyrings_len = keyrings_len; + } + entry->keyrings = kstrdup(args[0].from, GFP_KERNEL); if (!entry->keyrings) { + kfree(ima_keyrings); + ima_keyrings = NULL; + ima_keyrings_len = 0; result = -ENOMEM; break; } -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> To: zohar@linux.ibm.com, James.Bottomley@HansenPartnership.com, linux-integrity@vger.kernel.org Cc: dhowells@redhat.com, sashal@kernel.org, linux-kernel@vger.kernel.org, keyrings@vger.kernel.org, linux-crypto@vger.kernel.org Subject: [PATCH v1] IMA: pre-allocate buffer to hold keyrings string Date: Thu, 16 Jan 2020 23:46:23 +0000 [thread overview] Message-ID: <20200116234623.2959-1-nramas@linux.microsoft.com> (raw) ima_match_keyring() is called while holding rcu read lock. Since this function executes in atmomic context, it should not call any function that can sleep (such as kstrdup()). This patch pre-allocates a buffer to hold the keyrings string read from the IMA policy and uses that to match the given keyring. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Fixes: e9085e0ad38a ("IMA: Add support to limit measuring keys") --- security/integrity/ima/ima_policy.c | 50 ++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 9963863d6c92..180e2069e075 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -208,6 +208,10 @@ static LIST_HEAD(ima_policy_rules); static LIST_HEAD(ima_temp_rules); static struct list_head *ima_rules; +/* Pre-allocated buffer used for matching keyrings. */ +static char *ima_keyrings; +static size_t ima_keyrings_len; + static int ima_policy __initdata; static int __init default_measure_policy_setup(char *str) @@ -369,7 +373,7 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, static bool ima_match_keyring(struct ima_rule_entry *rule, const char *keyring, const struct cred *cred) { - char *keyrings, *next_keyring, *keyrings_ptr; + char *next_keyring, *keyrings_ptr; bool matched = false; if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid)) @@ -381,15 +385,13 @@ static bool ima_match_keyring(struct ima_rule_entry *rule, if (!keyring) return false; - keyrings = kstrdup(rule->keyrings, GFP_KERNEL); - if (!keyrings) - return false; + strcpy(ima_keyrings, rule->keyrings); /* * "keyrings=" is specified in the policy in the format below: * keyrings=.builtin_trusted_keys|.ima|.evm */ - keyrings_ptr = keyrings; + keyrings_ptr = ima_keyrings; while ((next_keyring = strsep(&keyrings_ptr, "|")) != NULL) { if (!strcmp(next_keyring, keyring)) { matched = true; @@ -397,8 +399,6 @@ static bool ima_match_keyring(struct ima_rule_entry *rule, } } - kfree(keyrings); - return matched; } @@ -949,6 +949,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) bool uid_token; struct ima_template_desc *template_desc; int result = 0; + size_t keyrings_len; ab = integrity_audit_log_start(audit_context(), GFP_KERNEL, AUDIT_INTEGRITY_POLICY_RULE); @@ -1114,14 +1115,47 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) case Opt_keyrings: ima_log_string(ab, "keyrings", args[0].from); + keyrings_len = strlen(args[0].from) + 1; + if ((entry->keyrings) || (entry->action != MEASURE) || - (entry->func != KEY_CHECK)) { + (entry->func != KEY_CHECK) || + (keyrings_len < 2)) { result = -EINVAL; break; } + + if (ima_keyrings) { + if (keyrings_len > ima_keyrings_len) { + char *tmpbuf; + + tmpbuf = krealloc(ima_keyrings, + keyrings_len, + GFP_KERNEL); + if (!tmpbuf) { + result = -ENOMEM; + break; + } + + ima_keyrings = tmpbuf; + ima_keyrings_len = keyrings_len; + } + } else { + ima_keyrings = kzalloc(keyrings_len, + GFP_KERNEL); + if (!ima_keyrings) { + result = -ENOMEM; + break; + } + + ima_keyrings_len = keyrings_len; + } + entry->keyrings = kstrdup(args[0].from, GFP_KERNEL); if (!entry->keyrings) { + kfree(ima_keyrings); + ima_keyrings = NULL; + ima_keyrings_len = 0; result = -ENOMEM; break; } -- 2.17.1
next reply other threads:[~2020-01-16 23:46 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-16 23:46 Lakshmi Ramasubramanian [this message] 2020-01-16 23:46 ` [PATCH v1] IMA: pre-allocate buffer to hold keyrings string Lakshmi Ramasubramanian 2020-01-17 1:17 ` Mimi Zohar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200116234623.2959-1-nramas@linux.microsoft.com \ --to=nramas@linux.microsoft.com \ --cc=James.Bottomley@HansenPartnership.com \ --cc=dhowells@redhat.com \ --cc=keyrings@vger.kernel.org \ --cc=linux-crypto@vger.kernel.org \ --cc=linux-integrity@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=sashal@kernel.org \ --cc=zohar@linux.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.