Re: [tpmdd-devel] [PATCH RFC v4 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c

[James Bottomley]

On Mon, 2017-01-23 at 16:30 -0700, Jason Gunthorpe wrote:
> On Mon, Jan 23, 2017 at 03:20:12PM -0800, James Bottomley wrote:
> 
> > > So you are saying there is so much already deployed TPM2 software
> > > that has this TPM_DEVICE env var convention that we need to 
> > > support it with compat?
> > > 
> > > I'm really surprised by that.. But OK.
> > > 
> > > Can you at least remove the 'user_read_timer' junk from the new
> > > cdev?
> > 
> > What's the problem with it?  Can we not just fix whatever the issue
> > is?
> 
> The issue is that it exists at all.
> 
> I've been unwilling to remove it because some crazy userspace might
> rely on it, but I really don't want to see it continue in any new
> stuff.

All it does is clear the pending read after 60s ... like you, I suspect
it could just be removed but I don't think having it present causes
problems.

> If you know the existing TPM1 userspace is safe then lets just delete
> it entirely. Otherwise lets be sure no new users crop up by disabling
> it.
> 
> > I'd rather reuse all the R/W machinery as is.  If I start trying to
> > special case it so that we only use some parts on some control 
> > flows, the chances are I'll introduce additional bugs as well.
> 
> Sure, this is part of the pain of compat..

Except for the added complexity and possibility of extra bugs, nothing
is gained by the special casing.  That tells me we should either remove
this interface behaviour globally or not at all.  Removing it globally
would be independent of the space patches, because they'd simply
inherit whatever was done.

Why don't you start by doubling the timeout?  If nothing notices,
chances are nothing relies on this aspect of the interface and it can
be easily removed.

James