linux-kernel - Re: [PATCH v2 3/3] tpm: in tpm2_del_space check if ops pointer is still valid

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: <995b242a-d8ec-9310-d272-960b3f025115@kunbus.com>
Date:   Wed, 3 Feb 2021 15:22:07 +0100
From:   Lino Sanfilippo <l.sanfilippo@...bus.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        Lino Sanfilippo <LinoSanfilippo@....de>
Cc:     peterhuewe@....de, jgg@...pe.ca, stefanb@...ux.vnet.ibm.com,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] tpm: in tpm2_del_space check if ops pointer is
 still valid

Hi,

On 03.02.21 02:17, Jarkko Sakkinen wrote:
> On Tue, Feb 02, 2021 at 11:09:03PM +0100, Lino Sanfilippo wrote:
>> From: Lino Sanfilippo <l.sanfilippo@...bus.com>
>>
>> In tpm2_del_space() the sessions are flushed by means of the tpm_chip
>> operations. However the concerning operations pointer my already be NULL at
>                                                         ~~
>                                                         may
> 
> What is "concerniog operations"? Unfamiliar term. Maybe just consistently
> se chip->ops? Now you have also "tpm_chip operations" and chip->ops, in
> addition to "concerning operations" in one paragraph commit message.

'concerning' referred to 'operations pointer'. But yes, using multiple times
 a different term for the same thing is quite confusing. I will fix this. 

>> this time in case that the chip has been unregistered (see
>> tpm_chip_unregister() which calls tpm_del_char_device() which sets
>> chip->ops to NULL).
>> Before accessing chip->ops check if it is still valid. Skip flushing
>> the sessions in this case.
>>
>> Signed-off-by: Lino Sanfilippo <l.sanfilippo@...bus.com>
> 
> Instead of cross referencing please describe the scenario (i.e.
> the sequence of operations) of failure.
> 
> Fixes tag is also missing.
> 

Right, will add it.

Thanks,
Lino