Subject: Re: [PATCH v2] tpm: fix potential NULL pointer access in
 tpm_del_char_device
To:     Stefan Berger <stefanb@linux.ibm.com>, peterhuewe@gmx.de,
        jarkko@kernel.org, jgg@ziepe.ca
Cc:     p.rosenberger@kunbus.com, linux-integrity@vger.kernel.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
References: <20211220150635.8545-1-LinoSanfilippo@gmx.de>
 <af847879-0f29-08e7-7609-da3b27381d3a@linux.ibm.com>
From:   Lino Sanfilippo <LinoSanfilippo@gmx.de>
Message-ID: <926f57a8-81b8-b3a9-8338-71213f1b85ac@gmx.de>
Date:   Wed, 22 Dec 2021 07:56:22 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <af847879-0f29-08e7-7609-da3b27381d3a@linux.ibm.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


Hi Stefan,


On 22.12.21 at 05:53, Stefan Berger wrote:

>>
>> =C2=A0 drivers/char/tpm/tpm-chip.c | 16 +++++++++++-----
>> =C2=A0 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index ddaeceb7e109..7960da490e72 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -474,13 +474,19 @@ static void tpm_del_char_device(struct tpm_chip *=
chip)
>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Make the driver uncallable. */
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 down_write(&chip->ops_sem);
>> -=C2=A0=C2=A0=C2=A0 if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!tpm_chip_start(chip)) =
{
>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tpm=
2_shutdown(chip, TPM2_SU_CLEAR);
>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tpm=
_chip_stop(chip);
>> +=C2=A0=C2=A0=C2=A0 /* Check if chip->ops is still valid: In case that =
the controller
>> +=C2=A0=C2=A0=C2=A0=C2=A0 * drivers shutdown handler unregisters the co=
ntroller in its
>> +=C2=A0=C2=A0=C2=A0=C2=A0 * shutdown handler we are called twice and ch=
ip->ops to NULL.
>> +=C2=A0=C2=A0=C2=A0=C2=A0 */
>> +=C2=A0=C2=A0=C2=A0 if (chip->ops) {
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (chip->flags & TPM_CHIP_=
FLAG_TPM2) {
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if =
(!tpm_chip_start(chip)) {
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 tpm2_shutdown(chip, TPM2_SU_CLEAR);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 tpm_chip_stop(chip);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 chip->ops =3D NULL;
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>> -=C2=A0=C2=A0=C2=A0 chip->ops =3D NULL;
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 up_write(&chip->ops_sem);
>> =C2=A0 }
>> =C2=A0
>> base-commit: a7904a538933c525096ca2ccde1e60d0ee62c08e
>
>
> Fixes: 39d0099f9439 ("powerpc/pseries: Add shutdown() to vio_driver and =
vio_bus")
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
>
>

Thanks a lot for testing this.

Best regards,
Lino