All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
@ 2022-03-07 10:48 GUO Zihua
  2022-03-07 13:45 ` Jarkko Sakkinen
  2022-03-14 16:37 ` Jason Gunthorpe
  0 siblings, 2 replies; 10+ messages in thread
From: GUO Zihua @ 2022-03-07 10:48 UTC (permalink / raw)
  To: linux-integrity
  Cc: wangweiyang2, xiujianfeng, weiyongjun1, peterhuewe, jarkko, jgg,
	linux-kernel

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@huawei.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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-07 10:48 [PATCH] tpm: Fix memory leak in tpmm_chip_alloc GUO Zihua
@ 2022-03-07 13:45 ` Jarkko Sakkinen
  2022-03-10  3:33   ` Guozihua (Scott)
  2022-03-14 16:37 ` Jason Gunthorpe
  1 sibling, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2022-03-07 13:45 UTC (permalink / raw)
  To: GUO Zihua
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jgg, linux-kernel

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@huawei.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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-07 13:45 ` Jarkko Sakkinen
@ 2022-03-10  3:33   ` Guozihua (Scott)
  2022-03-11 16:34     ` Jarkko Sakkinen
  0 siblings, 1 reply; 10+ messages in thread
From: Guozihua (Scott) @ 2022-03-10  3:33 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jgg, linux-kernel



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@huawei.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.

-- 
Best
GUO Zihua

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-10  3:33   ` Guozihua (Scott)
@ 2022-03-11 16:34     ` Jarkko Sakkinen
  2022-03-14  7:10       ` Guozihua (Scott)
  2022-03-15  3:11       ` Guozihua (Scott)
  0 siblings, 2 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2022-03-11 16:34 UTC (permalink / raw)
  To: Guozihua (Scott)
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jgg, linux-kernel

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@huawei.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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-11 16:34     ` Jarkko Sakkinen
@ 2022-03-14  7:10       ` Guozihua (Scott)
  2022-03-15  3:11       ` Guozihua (Scott)
  1 sibling, 0 replies; 10+ messages in thread
From: Guozihua (Scott) @ 2022-03-14  7:10 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jgg, linux-kernel



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@huawei.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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-07 10:48 [PATCH] tpm: Fix memory leak in tpmm_chip_alloc GUO Zihua
  2022-03-07 13:45 ` Jarkko Sakkinen
@ 2022-03-14 16:37 ` Jason Gunthorpe
  2022-03-15  1:55   ` Guozihua (Scott)
  2022-03-17  7:31   ` Jarkko Sakkinen
  1 sibling, 2 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2022-03-14 16:37 UTC (permalink / raw)
  To: GUO Zihua
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jarkko, linux-kernel

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@huawei.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
> +++ 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)

This looks like the same issue as was adressed by the recent discussion..

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-14 16:37 ` Jason Gunthorpe
@ 2022-03-15  1:55   ` Guozihua (Scott)
  2022-03-17  7:33     ` Jarkko Sakkinen
  2022-03-17  7:31   ` Jarkko Sakkinen
  1 sibling, 1 reply; 10+ messages in thread
From: Guozihua (Scott) @ 2022-03-15  1:55 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jarkko, linux-kernel



On 2022/3/15 0:37, Jason Gunthorpe 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@huawei.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
>> +++ 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)
> 
> This looks like the same issue as was adressed by the recent discussion..
> 
> Jason
> .

Hi Jason,

Would you mind refer me to the discussion?

-- 
Best
GUO Zihua

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-11 16:34     ` Jarkko Sakkinen
  2022-03-14  7:10       ` Guozihua (Scott)
@ 2022-03-15  3:11       ` Guozihua (Scott)
  1 sibling, 0 replies; 10+ messages in thread
From: Guozihua (Scott) @ 2022-03-15  3:11 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, wangweiyang2, xiujianfeng, weiyongjun1,
	peterhuewe, jgg, linux-kernel

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@huawei.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
> 
> 

Hi Jarkko,

The code on the master branch on 
git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git 
seems to be quite different from what I saw on linux upstream. Namely 
tpm_chip->devs does not exist on linux-tpmdd. Has this member been deleted?

-- 
Best
GUO Zihua

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-14 16:37 ` Jason Gunthorpe
  2022-03-15  1:55   ` Guozihua (Scott)
@ 2022-03-17  7:31   ` Jarkko Sakkinen
  1 sibling, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2022-03-17  7:31 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: GUO Zihua, linux-integrity, wangweiyang2, xiujianfeng,
	weiyongjun1, peterhuewe, linux-kernel

On Mon, Mar 14, 2022 at 01:37:05PM -0300, Jason Gunthorpe 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@huawei.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
> > +++ 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)
> 
> This looks like the same issue as was adressed by the recent discussion..

Both fixes (Lino, jejb) are also part of my PR:

[*] https://lore.kernel.org/linux-integrity/Yi64TJXqto+VdoOo@iki.fi/

> Jason

BR, Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: Fix memory leak in tpmm_chip_alloc
  2022-03-15  1:55   ` Guozihua (Scott)
@ 2022-03-17  7:33     ` Jarkko Sakkinen
  0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2022-03-17  7:33 UTC (permalink / raw)
  To: Guozihua (Scott)
  Cc: Jason Gunthorpe, linux-integrity, wangweiyang2, xiujianfeng,
	weiyongjun1, peterhuewe, linux-kernel

On Tue, Mar 15, 2022 at 09:55:07AM +0800, Guozihua (Scott) wrote:
> 
> 
> On 2022/3/15 0:37, Jason Gunthorpe 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@huawei.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
> > > +++ 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)
> > 
> > This looks like the same issue as was adressed by the recent discussion..
> > 
> > Jason
> > .
> 
> Hi Jason,
> 
> Would you mind refer me to the discussion?

Please test: git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

And use tag tpmdd-next-v5.18-v2

If issues persists, let us know or send a fix.

BR, Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-03-17  7:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 10:48 [PATCH] tpm: Fix memory leak in tpmm_chip_alloc GUO Zihua
2022-03-07 13:45 ` Jarkko Sakkinen
2022-03-10  3:33   ` Guozihua (Scott)
2022-03-11 16:34     ` Jarkko Sakkinen
2022-03-14  7:10       ` Guozihua (Scott)
2022-03-15  3:11       ` Guozihua (Scott)
2022-03-14 16:37 ` Jason Gunthorpe
2022-03-15  1:55   ` Guozihua (Scott)
2022-03-17  7:33     ` Jarkko Sakkinen
2022-03-17  7:31   ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.