| Message ID | 20220909011516.55957-2-guozihua@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | ima: Handle -ESTALE returned by ima_filter_rule_match() | expand |
Hi Scott, On Fri, 2022-09-09 at 09:15 +0800, GUO Zihua wrote: > Currently ima_lsm_copy_rule() set the arg_p field of the source rule to > NULL, so that the source rule could be freed afterward. It does not make > sense for this behavior to be inside a "copy" function. So move it > outside and let the caller handle this field. > > As of now, the only user of ima_lsm_copy_rule() is > ima_lsm_update_rule(). In ima_lsm_update_rule(), we would like to free > only the lsm.rule in the old rule entry, and leave arg_p untouch. In > this case, it's better to use ima_filter_rule_free() directly and not > introduce another for loop to set arg_p to NULL. > This needs to be re-written from a higher perspective. Perhaps base it on the existing comment in ima_lsm_update_rule(). ima_lsm_copy_rule() shallow copied all references, except for the LSM references. Only the LSM references and the entry itself need to be freed. Instead of calling ima_lsm_free_rule() to free the LSM references and also the args_p field, directly free the LSM references. > Signed-off-by: GUO Zihua <guozihua@huawei.com> > --- > security/integrity/ima/ima_policy.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index a8802b8da946..8040215c0252 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -398,12 +398,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) > > nentry->lsm[i].type = entry->lsm[i].type; > 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; > > ima_filter_rule_init(nentry->lsm[i].type, Audit_equal, > nentry->lsm[i].args_p, > @@ -417,6 +411,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) > > static int ima_lsm_update_rule(struct ima_rule_entry *entry) > { > + int i; > struct ima_rule_entry *nentry; > > nentry = ima_lsm_copy_rule(entry); > @@ -431,7 +426,8 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) > * references and the entry itself. All other memory references will now > * be owned by nentry. > */ > - ima_lsm_free_rule(entry); > + for (i = 0; i < MAX_LSM_RULES; i++) > + ima_filter_rule_free(entry->lsm[i].rule); > kfree(entry); > > return 0;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index a8802b8da946..8040215c0252 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -398,12 +398,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) nentry->lsm[i].type = entry->lsm[i].type; 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; ima_filter_rule_init(nentry->lsm[i].type, Audit_equal, nentry->lsm[i].args_p, @@ -417,6 +411,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) static int ima_lsm_update_rule(struct ima_rule_entry *entry) { + int i; struct ima_rule_entry *nentry; nentry = ima_lsm_copy_rule(entry); @@ -431,7 +426,8 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) * references and the entry itself. All other memory references will now * be owned by nentry. */ - ima_lsm_free_rule(entry); + for (i = 0; i < MAX_LSM_RULES; i++) + ima_filter_rule_free(entry->lsm[i].rule); kfree(entry); return 0;
Currently ima_lsm_copy_rule() set the arg_p field of the source rule to NULL, so that the source rule could be freed afterward. It does not make sense for this behavior to be inside a "copy" function. So move it outside and let the caller handle this field. As of now, the only user of ima_lsm_copy_rule() is ima_lsm_update_rule(). In ima_lsm_update_rule(), we would like to free only the lsm.rule in the old rule entry, and leave arg_p untouch. In this case, it's better to use ima_filter_rule_free() directly and not introduce another for loop to set arg_p to NULL. Signed-off-by: GUO Zihua <guozihua@huawei.com> --- security/integrity/ima/ima_policy.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)