linux-kernel - Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

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: <20210116115529.oq2k2qpgyawngcqn@liuwe-devbox-debian-v2>
Date:   Sat, 16 Jan 2021 11:55:29 +0000
From:   Wei Liu <wei.liu@...nel.org>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Wei Liu <wei.liu@...nel.org>,
        Linux Kernel List <linux-kernel@...r.kernel.org>,
        tyhicks@...ux.microsoft.com, "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Arnd Bergmann <arnd@...db.de>,
        Christian Gromm <christian.gromm@...rochip.com>
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> Hi,
> 
> On 1/15/21 4:12 PM, Wei Liu wrote:
> > For built-in drivers, the order of initialization function invocation is
> > determined by their link order.
> > 
> > The original code linked TPM drivers before TEE driver when they were
> > both built in. That caused fTPM's initialization to be deferred to a
> > worker thread instead of running on PID 1.
> > 
> > That is problematic because IMA's initialization routine, which runs on
> > PID 1 as a late initcall, needs to have access to the default TPM
> > instance. If fTPM's initialization is deferred, IMA will not be able to
> > get hold of a TPM instance in time.
> > 
> > Fix this by modifying Makefile to make sure TEE is initialized before
> > fTPM when they are both built in.
> > 
> > Signed-off-by: Wei Liu <wei.liu@...nel.org>
> > ---
> >  drivers/Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM)		+= nvmem/
> >  obj-$(CONFIG_FPGA)		+= fpga/
> >  obj-$(CONFIG_FSI)		+= fsi/
> >  obj-$(CONFIG_TEE)		+= tee/
> > +
> > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > +# deferred, which causes IMA to not get a TPM device in time
> > +obj-$(CONFIG_TCG_TPM)		+= char/tpm/
> > +
> >  obj-$(CONFIG_MULTIPLEXER)	+= mux/
> >  obj-$(CONFIG_UNISYS_VISORBUS)	+= visorbus/
> >  obj-$(CONFIG_SIOX)		+= siox/
> > 
> 
> As I suspected and then tested, since you did not remove the other build
> of char/tpm/, this ends up with multiple definition linker errors (below).

Oops, I didn't commit the hunk that removed the line in char/Makefile.

But I will hold off sending out v2 until the following discussion is
settled.

> 
> I would think that instead of depending on Makefile order you should use different
> initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> 

No, not really. The same trick was used in 2014 (1bacc894c227).

Both TEE and TPM are just drivers. I think they belong to the same level
(at the moment device_initcall).  Looking at the list of levels, I'm not
sure how I can move TEE to a different level.

Out of the seven levels, which one would you suggest I use for which
driver?

Wei.