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: <8039976be3df9bd07374fe4f1931b8ce28b89dab.camel@linux.ibm.com>
Subject: Re: [RFC PATCH 2/2] integrity: double check iint_cache was
 initialized
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        Dmitry Vyukov <dvyukov@google.com>
Cc:     linux-integrity@vger.kernel.org, James Morris <jmorris@namei.org>,
        "Serge E . Hallyn" <serge@hallyn.com>,
        linux-security-module <linux-security-module@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Eric Biggers <ebiggers@kernel.org>
Date:   Tue, 23 Mar 2021 08:09:21 -0400
In-Reply-To: <1a2245c6-3cab-7085-83d3-55b083619303@i-love.sakura.ne.jp>
References: <20210319200358.22816-1-zohar@linux.ibm.com>
         <20210319200358.22816-2-zohar@linux.ibm.com>
         <8450c80a-104a-3f36-0963-0ae8fa69e0f2@i-love.sakura.ne.jp>
         <CACT4Y+bvakfNhVs29QvbY6Z8Pw0zmAUKGWM-DD5DcPZW5ny90A@mail.gmail.com>
         <1a2245c6-3cab-7085-83d3-55b083619303@i-love.sakura.ne.jp>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2021-03-23 at 10:46 +0900, Tetsuo Handa wrote:
> On 2021/03/20 5:03, Mimi Zohar wrote:
> > The integrity's "iint_cache" is initialized at security_init().  Only
> > after an IMA policy is loaded, which is initialized at late_initcall,
> > is a file's integrity status stored in the "iint_cache".
> > 
> > All integrity_inode_get() callers first verify that the IMA policy has
> > been loaded, before calling it.  Yet for some reason, it is still being
> > called, causing a NULL pointer dereference.
> > 
> > qemu-system-x86_64 (...snipped...) lsm=smack (...snipped...)
> 
> Hmm, why are you using lsm=smack instead of security=smack ?
> Since use of lsm= overrides CONFIG_LSM="lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" settings,
> only smack is activated, which means that integrity_iintcache_init() will not be called by
> 
>   DEFINE_LSM(integrity) = {
>   	.name = "integrity",
>   	.init = integrity_iintcache_init,
>   };
> 
> declaration. That's the reason iint_cache == NULL when integrity_inode_get() is called.

That's exactly the problem, but since we don't control how the system
is configured or which parameters are supplied on the boot command
line, the kernel needs to at least provide some explanation instead of
dereferencing a NULL pointer.

FYI, "security=" is being deprecated.   From Documentation/admin-
guide/kernel-parameters.txt:

       security=  [SECURITY] Choose a legacy "major" security module to
                        enable at boot. This has been deprecated by the
                        "lsm=" parameter.

Please take a look at the newer version of this patch.   Do you want to
add any tags?

thanks,

Mimi