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;
Message-ID: <dc67b88df90ae037cb13d0ae4ba825ae891e5964.camel@linux.ibm.com>
Subject: Re: [PATCH] ima: fix infinite loop within "ima_match_policy"
 function.
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     =?UTF-8?Q?=E6=9D=8E=E5=8A=9B=E7=90=BC?= <liqiong@nfschina.com>,
        THOBY Simon <Simon.THOBY@viveris.fr>
Cc:     "dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
        "jmorris@namei.org" <jmorris@namei.org>,
        "serge@hallyn.com" <serge@hallyn.com>,
        "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
        "linux-security-module@vger.kernel.org" 
        <linux-security-module@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Date:   Mon, 23 Aug 2021 07:22:52 -0400
In-Reply-To: <f9798484-7090-0ddf-50a6-7c7c5bf0606c@nfschina.com>
References: <20210819101529.28001-1-liqiong@nfschina.com>
         <8d17f252-4a93-f430-3f25-e75556ab01e8@viveris.fr>
         <d385686b-ffa5-5794-2cf2-b87f2a471e78@nfschina.com>
         <1f631c3d-5dce-e477-bfb3-05aa38836442@viveris.fr>
         <96037695de6125c701889c168550def278adfd4b.camel@linux.ibm.com>
         <f9798484-7090-0ddf-50a6-7c7c5bf0606c@nfschina.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Liqiong,

On Mon, 2021-08-23 at 11:04 +0800, 李力琼 wrote:
> Hi Mimi :
> 
> The situation is a little different,'list_splice_init_rcu'
> don't change the list head. If "ima_rules" being changed,
> readers may can't reload the new value in time for cpu cache
> or compiler optimization. Defining "ima_rules" as a volatile 
> variable can fix, but It is inefficient.

After looking at list_splice_tail_init_rcu() some more, it is
actually making sure there aren't any readers traversing
"ima_temp_rules", not "ima_policy_rules".   There aren't any readers
traversing "ima_temp_rules". 

> 
> Maybe using a temporary ima_rules variable for every 
> "list_for_each_entry_rcu(entry, ima_rules, list)" loop is 
> a better solution to fix the "endless loop" bug. 

Agreed

thanks,

Mimi