linux-kernel - Re: [PATCH v2] tpm: Factor out common startup code

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: <20160518171320.GA15170@obsidianresearch.com>
Date:	Wed, 18 May 2016 11:13:20 -0600
From:	Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc:	andrew.zamansky@...oton.com, tpmdd-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v2] tpm: Factor out common startup code

On Wed, May 18, 2016 at 12:59:24PM +0300, Jarkko Sakkinen wrote:
> On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > > common startup sequence. This is the sequence used by tpm_tis and
> > > > tpm_crb.
> > > > 
> > > > All drivers should set this flag.
> > > 
> > > The commit message should be a much much more verbose I cannot include
> > > this without a better explanation. Please update this for the next
> > > revision.
> > 
> > What more description do you want to see?
> 
> It is lacking a lot of relevant information:
> 
> * It should explain what you mean by startup sequence".
> * It should describe the constant TPM_OPS_AUTO_STARTUP
> * It should explain what drivers are doing at the moment (before
>   this feature).
> * It should explain what is the benefit for different HW drivers
>   after applying this patch.
> * It should explain why you call the executed sequence "automatic"
>   and also use the word "standard". Yeah, I didn't understand this,
>   this not me being picky. Maybe it should be DEFAULT_STARTUP??

Well, use something like this then. You can edit descriptions to your
liking when you apply the patch, FWIW.

tpm: Factor out common startup code

The TCG standard startup sequence (get timeouts, tpm startup, etc) for
TPM and TPM2 chips is being open coded in many drivers, move it into
the core code.

tpm_tis and tpm_crb are used as the basis for the core code
implementation and the easy drivers are converted. In the process
several small drivers bugs relating to error handling this flow
are fixed.

For now the flag TPM_OPS_AUTO_STARTUP is optional to allow a staged
driver roll out, but ultimately all drivers should use this flow and
the flag removed. Some drivers still do not implement the startup
sequence at all and will need to be tested with it enabled.