lkml.org 
[lkml]   [2018]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/4] tpm: Implement tpm_chip_find() and tpm_chip_put() for other subsystems
From
Date
On 06/21/2018 01:56 PM, Jason Gunthorpe wrote:
> On Thu, Jun 21, 2018 at 01:45:03PM -0400, Stefan Berger wrote:
>> On 06/21/2018 01:15 PM, Jarkko Sakkinen wrote:
>>> On Wed, Jun 20, 2018 at 04:42:33PM -0400, Stefan Berger wrote:
>>>> Implement tpm_chip_find() for other subsystems to find a TPM chip and
>>>> get a reference to that chip. Once done with using the chip, the reference
>>>> is released using tpm_chip_put().
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>> You should sort this out in a way that we don't end up with duplicate
>>> functions.
>> Do you want me to create a function *like* tpm_chip_find_get() that takes an
>> additional parameter whether to get the ops semaphore and have that function
>> called by the existing tpm_chip_find_get() and the new tpm_chip_find(). The
>> latter would then not get the ops semphore. I didn't want to do this since
>> one time the function returns with a lock held and the other time not.
> Another option, and I haven't looked, is to revise the callers of
> tpm_chip_find_get to not require it to hold the ops semaphore for
> them.

We have tpm_chip_unregister calling tpm_del_char_device to set the ops
to NULL once a chip is unregistered. All existing callers, if they pass
in a tpm_chip != NULL, currently fail if the ops are NULL. (If they pass
in tpm_chip = NULL, they shouldn't find a chip once ops are null and it
has been removed from the IDR). I wouldn't change that since IMA will
call in with a tpm_chip != NULL and we want to protect the ops. All
existing code within the tpm subsystem does seem to call
tpm_chip_find_get() with a NULL pointer, though. Also trusted keys seems
to pass in a NULL pointer every time.

>
> Either by giving them an API to do it, or revising the TPM entry
> points to do it.
>
> I didn't look, but how did the ops semaphore get grabbed in your
> revised patches? They do grab it, right?

The revised patches do not touch the existing code much but will call
tpm_chip_find_get() and get that semaphore every time before the ops are
used. IMA is the only caller of tpm_chip_find() that now gets an
additional reference to the tpm_chip and these APIs get called like this
from IMA:

ima init: chip = tpm_chip_find()

ima::tpm: tpm_chip_find_get(chip) ... tpm_put_ops(chip)

ima::tpm: tpm_chip_find_get(chip) ... tpm_put_ops(chip)

[repeat]

ima shutdown: tpm_chip_put(chip)

    Stefan

>
> Jason
>

\
 
 \ /
  Last update: 2018-06-21 20:20    [W:0.101 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site