linux-kernel - Re: [PATCH] tpm: Fix memory leak in tpmm_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:   Mon, 14 Mar 2022 15:10:36 +0800
From:   "Guozihua (Scott)" <guozihua@...wei.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>
CC:     <linux-integrity@...r.kernel.org>, <wangweiyang2@...wei.com>,
        <xiujianfeng@...wei.com>, <weiyongjun1@...wei.com>,
        <peterhuewe@....de>, <jgg@...pe.ca>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc



On 2022/3/12 0:34, Jarkko Sakkinen wrote:
> On Thu, 2022-03-10 at 11:33 +0800, Guozihua (Scott) wrote:
>>
>>
>> On 2022/3/7 21:45, Jarkko Sakkinen wrote:
>>> On Mon, Mar 07, 2022 at 06:48:27PM +0800, GUO Zihua wrote:
>>>> Fix a memory leak in tpmm_chip_alloc. devm_add_action_or_reset would
>>>> call put_device on error, while tpm->devs is left untouched. Call
>>>> put_device on tpm->devs as well if devm_add_action_or_reset returns an
>>>> error.
>>>>
>>>> Fixes: fdc915f7f719 ("tpm: expose spaces via a device link /dev/tpmrm")
>>>> Signed-off-by: GUO Zihua <guozihua@...wei.com>
>>>> ---
>>>>    drivers/char/tpm/tpm-chip.c | 13 +++++++++----
>>>>    1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>>>> index b009e7479b70..0a92334e8c40 100644
>>>> --- a/drivers/char/tpm/tpm-chip.c
>>>> +++ b/drivers/char/tpm/tpm-chip.c
>>>> @@ -308,6 +308,12 @@ static int tpm_class_shutdown(struct device *dev)
>>>>          return 0;
>>>>    }
>>>>    
>>>> +static void tpm_chip_free(struct tpm_chip *chip)
>>>> +{
>>>> +       put_device(&chip->devs);
>>>> +       put_device(&chip->dev);
>>>> +}
>>>> +
>>>>    /**
>>>>     * tpm_chip_alloc() - allocate a new struct tpm_chip instance
>>>>     * @pdev: device to which the chip is associated
>>>> @@ -396,8 +402,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>>>>          return chip;
>>>>    
>>>>    out:
>>>> -       put_device(&chip->devs);
>>>> -       put_device(&chip->dev);
>>>> +       tpm_chip_free(chip);
>>>>          return ERR_PTR(rc);
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(tpm_chip_alloc);
>>>> @@ -420,8 +425,8 @@ struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
>>>>                  return chip;
>>>>    
>>>>          rc = devm_add_action_or_reset(pdev,
>>>> -                                     (void (*)(void *)) put_device,
>>>> -                                     &chip->dev);
>>>> +                                     (void (*)(void *)) tpm_chip_free,
>>>> +                                     chip);
>>>>          if (rc)
>>>>                  return ERR_PTR(rc);
>>>>    
>>>> -- 
>>>> 2.17.1
>>>>
>>>
>>> Please test against the latest in
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
>>>
>>> and share your results.
>>>
>>> BR, Jarkko
>>> .
>>
>> Hi Jarkko,
>>
>> I'll do that. Do we have a test set for TPM? Or do we just build and run
>> it and see if everything works as expected?
>>
>> This is an error handling optimization BTW.
> 
> There is kselftest in tools/testing/kselftes/tpm2 that you can use
> but do not have to.
> 
> BR, Jarkko
> 
> 

Great! Thanks Jarkko!

-- 
Best
GUO Zihua