linux-kernel - Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown

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: <20170117205933.GA9604@obsidianresearch.com>
Date:   Tue, 17 Jan 2017 13:59:33 -0700
From:   Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:     Andrey Pronin <apronin@...omium.org>
Cc:     Peter Huewe <peterhuewe@....de>,
        Marcel Selhorst <tpmdd@...horst.net>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        semenzato@...omium.org, groeck@...omium.org
Subject: Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on
 shutdown

On Tue, Jan 17, 2017 at 12:13:36PM -0800, Andrey Pronin wrote:
> > Is there some way we can have the TPM core do this without requiring
> > the driver to add a shutdown the struct driver?
> > 
> > Maybe we could put something in chip->dev->driver? Not sure..
> 
> I can play more with it. We can check in tpm_chip_register() if
> chip->dev->driver->shutdown is NULL, and, if so, set it to a default
> handler. Or, do register_reboot_notifier() instead, to avoid messing
> with struct device_driver from tpm-chip.c. Not sure if that's a
> consideration at alli - any reason not to mess with those structures?

I think ordering is important here, the TPM core has to do any
shutdown before the driver shutdown method. That restriction might
entirely preclude using a reboot_notifier.

> Whatever we do, we should allow the drivers to still send
> (vendor-specific) commands from their shutdown handlers.

A vendor specific command should be done via a new core TPM
mechanism. I really want to keep access drivers (eg i2c, lpc, spi,
etc) out of the buisness of *assuming* they are connected to any
specific chip.

So, the core should detect chip XYZ and then issue the required
vendor-specific command in some way.

The driver shutdown would be used to close the access interface in
some way.

> But, yes, setting a default handler through chip->dev->driver
> might just be good enough.

Probably the *best* thing would be to add shutdown to 'struct class'
in the driver core like suspend/resume?

Jason