archive mirror
 help / color / mirror / Atom feed
From: <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
To: jarkko.sakkinen-VuQAYsv1563Yd54FQh9/
Subject: Re: [PATCH RESEND 3/3] tpm-chip: Export TPM device to user space even when startup failed
Date: Mon, 28 Aug 2017 17:15:58 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20170825172021.lw3ycxqw63ubrcm2-VuQAYsv1563Yd54FQh9/>

> On Thu, Aug 24, 2017 at 10:37:14AM +0200, Alexander Steffen wrote:
> > When one of the commands during the auto_startup sequences does not
> > return TPM_RC_SUCCESS, tpm_chip_register misleadingly returns
> > even though a TPM device is definitely present.
> >
> > An error response during those sequences is indeed unexpected, so to
> > prevent subsequent errors, the kernel should not make use of the TPM
> > device. But user space applications still might be able to communicate
> > with the TPM, so they can be used to further diagnose and/or fix the
> > problem. To allow this, with this patch the device is still exported
> > to user space, even if a TPM error code has been received, but the
> > kernel itself will not be allowed to use the device for anything.
> >
> > This is not a hypothetical scenario, but there are devices in the wild
> > that show this behavior. With this fix, those devices can be recovered
> > from their failed state.
> >
> > Signed-off-by: Alexander Steffen <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
> I can understand the benefits but you could make the same argument for
> any class devices that kernel handles.

In a similar situation I probably would make that argument for other devices as well :-)

> I don't think it is that common to let user space access malfunctioning
> devices.

But is that just because nobody bothered to implement the necessary logic or for some other reason?

In my opinion, it strongly depends on the effects of the malfunction. If the device does not respond at all or returns only garbage, it is of course not useful to make it available. But the case that I want to handle here has a TPM that communicates correctly, i.e. the TIS layer works fine, it just does not return the application-level responses that the kernel expects. I'd like to think of it more as a transparent communication channel that the kernel provides to user space applications, without interfering with the data that is transmitted, similar to how a TCP socket is a transparent channel, that does not care about HTTP 500 codes being transmitted. So it is okay for the kernel to refuse to use such a device internally (the kernel knows there is nothing it can do with a broken device),
  but why assume that no user space application can use it either?

I'd rather argue that user space applications already need to be prepared to handle TPM errors/malfunctions at any time, so removing this one check does not put them into a worse position. The TPM driver just checks *at startup* that the TPM seems to be okay (i.e. executes all self tests correctly). But if you keep your machine running for two years the TPM may fail at any point and the driver never checks it again, so an application may never assume that simply because /dev/tpm0 exists it can send arbitrary commands there that will always work (in fact, such a look-before-you-leap approach can never be guaranteed to succeed).

Also, what would be the alternative? Your TPM is broken in a way that the kernel does not export it, how do you debug/fix the problem? You cannot even execute a TPM2_GetTestResult to get more detailed error information. So at the moment, we have user space applications bypassing the kernel driver completely, by accessing /dev/mem and reimplementing all the TIS logic. But this is just duplicated code (with its own set of problems, for example, if the kernel driver is active at the same time, there is nothing that prevents concurrent accesses), so nobody wants to implement a similar solution for SPI, and I2C, and all the other interfaces that might be necessary in the future.

> PS. You should have in *all* patches that you don't tag as RFC have linux-
> kernel-u79uwXL29TaiAVqoAR/ Now you have it in *none* of your patches.
> When you don't have RFC you are saying out loud that this is production
> ready code that should be included to the mainline kernel.
> PPS. This patch set should be obviously RFC. It's  avery questionable and
> intrusive change. That's why I didn't include linux-kernel.

Sorry, thanks for explaining, will do that next time.

Check out the vibrant tech community on one of the world's most
engaging tech sites,!

  parent reply	other threads:[~2017-08-28 17:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-24  8:37 [PATCH RESEND 0/3] Export broken TPMs to user space Alexander Steffen
     [not found] ` <20170824083714.10016-1-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
2017-08-24  8:37   ` [PATCH RESEND 1/3] tpm-chip: Move idr_replace calls to appropriate places Alexander Steffen
2017-08-25 17:25     ` Jarkko Sakkinen
     [not found]       ` <20170825172546.f4bl2wh7tgbyjx2n-VuQAYsv1563Yd54FQh9/>
2017-08-28 17:18         ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
2017-08-24  8:37   ` [PATCH RESEND 2/3] tpm-chip: Return TPM error codes from auto_startup functions Alexander Steffen
     [not found]     ` <20170824083714.10016-3-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
2017-08-25 17:06       ` Jarkko Sakkinen
     [not found]         ` <20170825170607.wfnr5y5zres2n42r-VuQAYsv1563Yd54FQh9/>
2017-08-29 12:11           ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
2017-08-24  8:37   ` [PATCH RESEND 3/3] tpm-chip: Export TPM device to user space even when startup failed Alexander Steffen
2017-08-25 17:20     ` Jarkko Sakkinen
     [not found]       ` <20170825172021.lw3ycxqw63ubrcm2-VuQAYsv1563Yd54FQh9/>
2017-08-28 17:15         ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w [this message]
2017-08-29 12:55           ` Jarkko Sakkinen
2017-08-29 13:17             ` [tpmdd-devel] " Michal Suchánek
     [not found]               ` <>
2017-08-29 13:53                 ` Peter Huewe
2017-08-30 10:26                   ` [tpmdd-devel] " Jarkko Sakkinen
2017-08-30 10:15                 ` Jarkko Sakkinen
2017-08-30 10:20                   ` [tpmdd-devel] " Jarkko Sakkinen
2017-08-30 10:34                     ` Michal Suchánek
2017-08-30 11:07                       ` Jarkko Sakkinen
2017-08-31 16:18                         ` Alexander.Steffen
2017-09-02 10:20                           ` Jarkko Sakkinen
     [not found]                   ` <20170830101510.rlkh2p3zecfsrhgl-VuQAYsv1563Yd54FQh9/>
2017-08-30 10:41                     ` Peter Huewe
2017-08-30 11:10                       ` [tpmdd-devel] " Jarkko Sakkinen
2017-08-31 16:26                         ` Alexander.Steffen
2017-09-02 10:24                           ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \
    --to=alexander.steffen-d0qzbvysippwk0htik3j/ \
    --cc=jarkko.sakkinen-VuQAYsv1563Yd54FQh9/ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).