Received: by 2002:a05:6902:102b:0:0:0:0 with SMTP id x11csp1153340ybt; Wed, 8 Jul 2020 23:20:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz8EL7T3f/Ma8PmgI4q2xGAfa+/n2nIXdI06d2eqN30YaBHniJYk6QnUh577puw9GMyavY6 X-Received: by 2002:a50:ee84:: with SMTP id f4mr68543700edr.183.1594275643016; Wed, 08 Jul 2020 23:20:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1594275643; cv=none; d=google.com; s=arc-20160816; b=sY3TINBoqtlfNgp0IN5Afu0IHYl3Eh11AIU47bQYs1ODPI11+qajC7xDEr/vWV+Ie0 QCK+Z7v1mjA3abVT9oVLB/PBPcpPXNXt5YYQnL1Ksi0vK27v3DgmrKgLWSasPnPihTOA DZd/xPy3+Q2mRnwd4MlLRH+V2lZ0NvpO5MAs06orFYWlVlMhYYpPXcK+INuqg+mLfBlp S6MCvJNR1l5olBmeAXR/mKKk5z15LlQ+kQvQ3BV4tF+MumJIczqfTS0GSml0MyUpOX4I r7iyHdE5I+/kSUjZJU3IEOT2+igSiU7lip3At0lTz+FNzSNl4H5kmr8lzG0RlQ1vrquv xGaQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:dkim-filter; bh=eIE8kXA186NO1r5n/4ptF6EogSRK2Bt/9uDqsHqZ/7E=; b=bUXzhPy/uPSoObAuMkNEcOeAkXqbmnWTeXYFEdMzhvSa1SXJ2DXOS+v7ssEBkK4RvD V8zMYR+eY60K4ad+InCO5XYPsFMeqZX1bsljZ3engRLeOh/FTJTvuYdsthwn495RY56D zkC6XWgC7Y45sBmykvQYBPe5HG8kRrHB5u7wjUh/MC5q5F7yC+MNqC865x7KIl9M8R+v ix74aMoePT9Dvbs4zI/+q8agxbrgqg5Qstr++a7erB9kKeoZZXNMKZV8h/nhEPPXBT7F nNvcff52J/JDq2LinkZ4rASZB+b7pQJd/iP0HBeROxyatn9zWTv3pa6505zK/f+4JDBi NLOA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=YOmyAKXV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id m27si1524258edj.431.2020.07.08.23.20.17; Wed, 08 Jul 2020 23:20:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=YOmyAKXV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726336AbgGIGUA (ORCPT + 99 others); Thu, 9 Jul 2020 02:20:00 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37938 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgGIGT6 (ORCPT ); Thu, 9 Jul 2020 02:19:58 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id CA21A20B4909; Wed, 8 Jul 2020 23:19:56 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com CA21A20B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275597; bh=eIE8kXA186NO1r5n/4ptF6EogSRK2Bt/9uDqsHqZ/7E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YOmyAKXV8UChPB689eZr6B/Yd8Wn28MDRLTbiEEQGiOVzdwMBhq60LKzQac2ah3JW dK2Hwov48aj/rB47nUWYLOXcoT9+06sJghtFZ77EOmmHu3T2tzguNb8Yrmv/DTWXwz hbBGzQNg1lvHYhcXAu8VdKMjxKn/FahDpz8Q4go4= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 02/12] ima: Free the entire rule when deleting a list of rules Date: Thu, 9 Jul 2020 01:19:01 -0500 Message-Id: <20200709061911.954326-3-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Create a function, ima_free_rule(), to free all memory associated with an ima_rule_entry. Use the new function to fix memory leaks of allocated ima_rule_entry members, such as .fsname and .keyrings, when deleting a list of rules. Make the existing ima_lsm_free_rule() function specific to the LSM audit rule array of an ima_rule_entry and require that callers make an additional call to kfree to free the ima_rule_entry itself. This fixes a memory leak seen when loading by a valid rule that contains an additional piece of allocated memory, such as an fsname, followed by an invalid rule that triggers a policy load failure: # echo -e "dont_measure fsname=securityfs\nbad syntax" > \ /sys/kernel/security/ima/policy -bash: echo: write error: Invalid argument # echo scan > /sys/kernel/debug/kmemleak # cat /sys/kernel/debug/kmemleak unreferenced object 0xffff9bab67ca12c0 (size 16): comm "bash", pid 684, jiffies 4295212803 (age 252.344s) hex dump (first 16 bytes): 73 65 63 75 72 69 74 79 66 73 00 6b 6b 6b 6b a5 securityfs.kkkk. backtrace: [<00000000adc80b1b>] kstrdup+0x2e/0x60 [<00000000d504cb0d>] ima_parse_add_rule+0x7d4/0x1020 [<00000000444825ac>] ima_write_policy+0xab/0x1d0 [<000000002b7f0d6c>] vfs_write+0xde/0x1d0 [<0000000096feedcf>] ksys_write+0x68/0xe0 [<0000000052b544a2>] do_syscall_64+0x56/0xa0 [<000000007ead1ba7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: f1b08bbcbdaf ("ima: define a new policy condition based on the filesystem name") Fixes: 2b60c0ecedf8 ("IMA: Read keyrings= option from the IMA policy") Signed-off-by: Tyler Hicks --- * v3 - No change * v2 - Collapsed patch #2 from v1 of this series, into this patch. This patch now introduces ima_free_rule(). - Existing callers of ima_lsm_free_rule() are doing so to free rules after a successful or failed ima_lsm_copy_rule() and those callers continue to directly call ima_lsm_copy_rule() rather than doing explicit reference ownership and calling ima_free_rule(). - The kfree(entry) of ima_lsm_free_rule() was removed from that function to make it focused on freeing the LSM references. Direct callers of ima_lsm_free_rule() must now call kfree(entry) after ima_lsm_free_rule(). - A comment was added in ima_lsm_update_rule() to clarify why ima_free_rule() isn't being used. security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index d7c268c2b0ce..bf00b966e87f 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -261,6 +261,21 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry) security_filter_rule_free(entry->lsm[i].rule); kfree(entry->lsm[i].args_p); } +} + +static void ima_free_rule(struct ima_rule_entry *entry) +{ + if (!entry) + return; + + /* + * entry->template->fields may be allocated in ima_parse_rule() but that + * reference is owned by the corresponding ima_template_desc element in + * the defined_templates list and cannot be freed here + */ + kfree(entry->fsname); + kfree(entry->keyrings); + ima_lsm_free_rule(entry); kfree(entry); } @@ -302,6 +317,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) out_err: ima_lsm_free_rule(nentry); + kfree(nentry); return NULL; } @@ -315,7 +331,14 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) list_replace_rcu(&entry->list, &nentry->list); synchronize_rcu(); + /* + * ima_lsm_copy_rule() shallow copied all references, except for the + * LSM references, from entry to nentry so we only want to free the LSM + * references and the entry itself. All other memory refrences will now + * be owned by nentry. + */ ima_lsm_free_rule(entry); + kfree(entry); return 0; } @@ -1402,15 +1425,11 @@ ssize_t ima_parse_add_rule(char *rule) void ima_delete_rules(void) { struct ima_rule_entry *entry, *tmp; - int i; temp_ima_appraise = 0; list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) { - for (i = 0; i < MAX_LSM_RULES; i++) - kfree(entry->lsm[i].args_p); - list_del(&entry->list); - kfree(entry); + ima_free_rule(entry); } } -- 2.25.1