linux-kernel - Re: [PATCH v2 1/4] tpm: Implement tpm_chip_find() and tpm_chip

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]

Date:   Thu, 21 Jun 2018 16:14:46 -0400
From:   Stefan Berger <stefanb@...ux.vnet.ibm.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        linux-integrity@...r.kernel.org, zohar@...ux.vnet.ibm.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] tpm: Implement tpm_chip_find() and tpm_chip_put()
 for other subsystems

On 06/21/2018 03:06 PM, Jason Gunthorpe wrote:
> On Thu, Jun 21, 2018 at 02:19:44PM -0400, Stefan Berger wrote:
>> 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@...ux.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)
> Maybe just change tpm_chip_find_get() into tpm_get_ops(chip) and
> convert all callers?

And then re-introduce tpm_chip_find_get() for IMA to call ?