On 6/13/2013 8:29 AM, Tomasz Stanislawski wrote:
> The smack_parsed_rule structure is allocated. If a rule is successfully
> installed then the last reference to the object is lost. This patch fixes
> this
> leak. Moreover smack_parsed_rule is allocated on stack because it no longer
> needed ofter smk_write_rules_list() is finished.
>
> Signed-off-by: Tomasz Stanislawski <t.stanisl...@samsung.com>
It looks like this was introduced with the change-rule support.
Prior to that, the rule passed into smk_set_access() was added
to the rule list if it was new. The change-rule support added a
new structure and missed the fact the rule was already allocated.
The patch needs to be rebased so that it does not depend on the
changes from patches 1 and 2 of the set.
> ---
> security/smack/smackfs.c | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 46f111e..e8c57f3 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -447,7 +447,7 @@ static ssize_t smk_write_rules_list(struct file *file,
> const char __user *buf,
> struct mutex *rule_lock, int format)
> {
> struct smack_known *skp;
> - struct smack_parsed_rule *rule;
> + struct smack_parsed_rule rule;
> char data[SMK_LOAD2LEN + 1];
> int rc = -EINVAL;
> int load = 0;
> @@ -475,49 +475,39 @@ static ssize_t smk_write_rules_list(struct file *file,
> const char __user *buf,
> goto out;
> }
>
> - rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> - if (rule == NULL) {
> - rc = -ENOMEM;
> - goto out;
> - }
> -
> if (format == SMK_LONG_FMT) {
> /*
> * Be sure the data string is terminated.
> */
> data[count] = '\0';
> - if (smk_parse_long_rule(data, rule, 1, 0))
> - goto out_free_rule;
> + if (smk_parse_long_rule(data, &rule, 1, 0))
> + goto out;
> } else if (format == SMK_CHANGE_FMT) {
> data[count] = '\0';
> - if (smk_parse_long_rule(data, rule, 1, 1))
> - goto out_free_rule;
> + if (smk_parse_long_rule(data, &rule, 1, 1))
> + goto out;
> } else {
> /*
> * More on the minor hack for backward compatibility
> */
> if (count == (SMK_OLOADLEN))
> data[SMK_OLOADLEN] = '-';
> - if (smk_parse_rule(data, rule, 1))
> - goto out_free_rule;
> + if (smk_parse_rule(data, &rule, 1))
> + goto out;
> }
>
>
> if (rule_list == NULL) {
> load = 1;
> - skp = smk_find_entry(rule->smk_subject);
> + skp = smk_find_entry(rule.smk_subject);
> rule_list = &skp->smk_rules;
> rule_lock = &skp->smk_rules_lock;
> }
>
> - rc = smk_set_access(rule, rule_list, rule_lock, load);
> - if (rc == 0) {
> + rc = smk_set_access(&rule, rule_list, rule_lock, load);
> + if (rc == 0)
> rc = count;
> - goto out;
> - }
>
> -out_free_rule:
> - kfree(rule);
> out:
> return rc;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
