Re: [PATCH v3 08/12] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements
From: Konsta Karsisto <hidden>
Date: 2020-07-17 15:35:17
Also in:
linux-integrity, lkml
Hi, Found one glitch with this change, see below: On Thu, Jul 9, 2020 at 9:22 AM Tyler Hicks [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The args_p member is a simple string that is allocated by ima_rule_init(). Shallow copy it like other non-LSM references in ima_rule_entry structs. There are no longer any necessary error path cleanups to do in ima_lsm_copy_rule(). Signed-off-by: Tyler Hicks <redacted> --- * v3 - No change * v2 - Adjusted context to account for ima_lsm_copy_rule() directly calling ima_lsm_free_rule() and the lack of explicit reference ownership transfers - Added comment to ima_lsm_copy_rule() to document the args_p reference ownership transfer security/integrity/ima/ima_policy.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 9842e2e0bc6d..b02e1ffd10c9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c@@ -300,10 +300,13 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) continue; nentry->lsm[i].type = entry->lsm[i].type; - nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p, - GFP_KERNEL); - if (!nentry->lsm[i].args_p) - goto out_err; + nentry->lsm[i].args_p = entry->lsm[i].args_p; + /* + * Remove the reference from entry so that the associated + * memory will not be freed during a later call to + * ima_lsm_free_rule(entry). + */ + entry->lsm[i].args_p = NULL;
This assignment necessitates a change in the code below...
quoted hunk ↗ jump to hunk
security_filter_rule_init(nentry->lsm[i].type, Audit_equal,@@ -314,11 +317,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) (char *)entry->lsm[i].args_p);
... you should refer to nentry->lsm[i].args_p here! Other than that, Reviewed-by: Konsta Karsisto <redacted> Konsta
}
return nentry;
-
-out_err:
- ima_lsm_free_rule(nentry);
- kfree(nentry);
- return NULL;
}
static int ima_lsm_update_rule(struct ima_rule_entry *entry)
--
2.25.1