Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965123AbbLVWLt (ORCPT ); Tue, 22 Dec 2015 17:11:49 -0500 Received: from lan.nucleusys.com ([92.247.61.126]:46404 "EHLO zztop.nucleusys.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933068AbbLVWLs (ORCPT ); Tue, 22 Dec 2015 17:11:48 -0500 X-Greylist: delayed 1894 seconds by postgrey-1.27 at vger.kernel.org; Tue, 22 Dec 2015 17:11:47 EST User-Agent: K-9 Mail for Android In-Reply-To: <1450814188.2774.9.camel@linux.vnet.ibm.com> References: <1450792283-8702-1-git-send-email-sasha.levin@oracle.com> <1450814188.2774.9.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH] IMA: policy can be updated zero times From: Petko Manolov Date: Tue, 22 Dec 2015 23:40:03 +0200 To: Mimi Zohar , Sasha Levin CC: dmitry.kasatkin@gmail.com, james.l.morris@oracle.com, serge@hallyn.com, linux-ima-devel@lists.sourceforge.net, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: <31A92AAB-5516-4176-A947-128CB65B6931@mip-labs.com> X-Spam-Score: -1.0 (-) X-Spam-Report: Spam detection software, running on the system "zztop.nucleusys.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: On December 22, 2015 9:56:28 PM GMT+02:00, Mimi Zohar wrote: >On Tue, 2015-12-22 at 08:51 -0500, Sasha Levin wrote: >> Commit "IMA: policy can now be updated multiple times" assumed that >the >> policy would be updated at least once. >> >> If there are zero updates, the temporary list head object will get >added >> to the policy list, and later dereferenced as an IMA policy object, >which >> means that invalid memory will be accessed. >> >> Signed-off-by: Sasha Levin >> --- >> security/integrity/ima/ima_policy.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/security/integrity/ima/ima_policy.c >b/security/integrity/ima/ima_policy.c >> index ba5d2fc..9b958b8 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -431,6 +431,9 @@ void ima_update_policy(void) >> { >> struct list_head *first, *last, *policy; >> >> + if (list_empty(&ima_temp_rules)) >> + return; >> + >> /* append current policy with the new rules */ >> first = (&ima_temp_rules)->next; >> last = (&ima_temp_rules)->prev; > >Thanks, Sasha. By the time ima_update_policy() is called >ima_release_policy() has already output the policy update status >message. I guess an empty policy could be considered a valid policy. >Could you add a msg indicating that the new policy was empty? [...] Content analysis details: (-1.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1880 Lines: 53 On December 22, 2015 9:56:28 PM GMT+02:00, Mimi Zohar wrote: >On Tue, 2015-12-22 at 08:51 -0500, Sasha Levin wrote: >> Commit "IMA: policy can now be updated multiple times" assumed that >the >> policy would be updated at least once. >> >> If there are zero updates, the temporary list head object will get >added >> to the policy list, and later dereferenced as an IMA policy object, >which >> means that invalid memory will be accessed. >> >> Signed-off-by: Sasha Levin >> --- >> security/integrity/ima/ima_policy.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/security/integrity/ima/ima_policy.c >b/security/integrity/ima/ima_policy.c >> index ba5d2fc..9b958b8 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -431,6 +431,9 @@ void ima_update_policy(void) >> { >> struct list_head *first, *last, *policy; >> >> + if (list_empty(&ima_temp_rules)) >> + return; >> + >> /* append current policy with the new rules */ >> first = (&ima_temp_rules)->next; >> last = (&ima_temp_rules)->prev; > >Thanks, Sasha. By the time ima_update_policy() is called >ima_release_policy() has already output the policy update status >message. I guess an empty policy could be considered a valid policy. >Could you add a msg indicating that the new policy was empty? As far as I can say we can't get to ima_update_policy() with empty ima_temp_rules because ima_write_policy() will set valid_policy to 0 in case of an empty rule. I'll double check it tomorrow, but please you do that too. cheers, Petko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/