linux-kernel - Re: [RFC PATCH 2/2] integrity: double check iint

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8039976be3df9bd07374fe4f1931b8ce28b89dab.camel@linux.ibm.com>
Date:   Tue, 23 Mar 2021 08:09:21 -0400
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Dmitry Vyukov <dvyukov@...gle.com>
Cc:     linux-integrity@...r.kernel.org, James Morris <jmorris@...ei.org>,
        "Serge E . Hallyn" <serge@...lyn.com>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Eric Biggers <ebiggers@...nel.org>
Subject: Re: [RFC PATCH 2/2] integrity: double check iint_cache was
 initialized

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