[PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Dongliang Mu]

The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
double free reported by Smatch") incorrectly handle the deallocation of
res variable. As shown in the comment, intel_vsec_add_aux handles all
the deallocation of res and feature_vsec_dev. Therefore, kfree(res) can
still cause double free if intel_vsec_add_aux returns error.

Fix this by adjusting the error handling part in tpmi_create_device,
following the function intel_vsec_add_dev.

Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free reported by Smatch")
Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
 drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index c999732b0f1e..882fe5e4763f 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -215,8 +215,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
 
 	feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev), GFP_KERNEL);
 	if (!feature_vsec_dev) {
-		ret = -ENOMEM;
-		goto free_res;
+		kfree(res);
+		return -ENOMEM;
 	}
 
 	snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-%s", name);
@@ -242,17 +242,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
 	 * feature_vsec_dev memory is also freed as part of device
 	 * delete.
 	 */
-	ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
-				 feature_vsec_dev, feature_id_name);
-	if (ret)
-		goto free_res;
-
-	return 0;
-
-free_res:
-	kfree(res);
-
-	return ret;
+	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
+				  feature_vsec_dev, feature_id_name);
 }
 
 static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)
-- 
2.39.2

[PATCH linux-next v2 2/3] drivers/platform/x86/intel/tpmi: revise the comment of intel_vsec_add_aux

[Dongliang Mu]

intel_vsec_add_aux() is resource managed including res and
feature_vsec_dev memory.

Fix this by revising the comment of intel_vsec_add_aux since res variable
will also be freed in the intel_vsec_add_aux.

Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
 drivers/platform/x86/intel/tpmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 882fe5e4763f..036d0e0dba19 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -239,8 +239,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
 	/*
 	 * intel_vsec_add_aux() is resource managed, no explicit
 	 * delete is required on error or on module unload.
-	 * feature_vsec_dev memory is also freed as part of device
-	 * delete.
+	 * feature_vsec_dev and res memory are also freed as part of
+	 * device deletion.
 	 */
 	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
 				  feature_vsec_dev, feature_id_name);
-- 
2.39.2

[PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak in intel_vsec_add_aux

[Dongliang Mu]

The first error handling code in intel_vsec_add_aux misses the
deallocation of intel_vsec_dev->resource.

Fix this by adding kfree(intel_vsec_dev->resource) in the error handling
code.

Reviewed-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
 drivers/platform/x86/intel/vsec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 13decf36c6de..2311c16cb975 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -154,6 +154,7 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
 	ret = ida_alloc(intel_vsec_dev->ida, GFP_KERNEL);
 	mutex_unlock(&vsec_ida_lock);
 	if (ret < 0) {
+		kfree(intel_vsec_dev->resource);
 		kfree(intel_vsec_dev);
 		return ret;
 	}
-- 
2.39.2

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Dan Carpenter]

On Thu, Mar 09, 2023 at 12:01:05PM +0800, Dongliang Mu wrote:
> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
> double free reported by Smatch") incorrectly handle the deallocation of
> res variable. As shown in the comment, intel_vsec_add_aux handles all
> the deallocation of res and feature_vsec_dev. Therefore, kfree(res) can
> still cause double free if intel_vsec_add_aux returns error.
> 
> Fix this by adjusting the error handling part in tpmi_create_device,
> following the function intel_vsec_add_dev.
> 
> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free reported by Smatch")
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
> ---

Yeah.  These patches are right.  The earlier fix still has a double
free.  Devres stuff is confusing...

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Hans de Goede]

Hi,

On 3/9/23 05:01, Dongliang Mu wrote:
> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
> double free reported by Smatch") incorrectly handle the deallocation of
> res variable. As shown in the comment, intel_vsec_add_aux handles all
> the deallocation of res and feature_vsec_dev. Therefore, kfree(res) can
> still cause double free if intel_vsec_add_aux returns error.
> 
> Fix this by adjusting the error handling part in tpmi_create_device,
> following the function intel_vsec_add_dev.
> 
> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free reported by Smatch")
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

IIRC then after this v2 was posted I still saw some comments on the original v1 which was not posted on the list. Without the v1 comments being on the list and this archived, I have lost track of what the status of these patches is.

Srinivas, can you let me know if I should merge these, or if more changes are necessary ?

From the off-list discussion of v1 I got the impression more changes are necessary, but I'm not sure.

Regards,

Hans




> ---
>  drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
> index c999732b0f1e..882fe5e4763f 100644
> --- a/drivers/platform/x86/intel/tpmi.c
> +++ b/drivers/platform/x86/intel/tpmi.c
> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>  
>  	feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev), GFP_KERNEL);
>  	if (!feature_vsec_dev) {
> -		ret = -ENOMEM;
> -		goto free_res;
> +		kfree(res);
> +		return -ENOMEM;
>  	}
>  
>  	snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-%s", name);
> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>  	 * feature_vsec_dev memory is also freed as part of device
>  	 * delete.
>  	 */
> -	ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
> -				 feature_vsec_dev, feature_id_name);
> -	if (ret)
> -		goto free_res;
> -
> -	return 0;
> -
> -free_res:
> -	kfree(res);
> -
> -	return ret;
> +	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
> +				  feature_vsec_dev, feature_id_name);
>  }
>  
>  static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[srinivas pandruvada]

Hi Hans,

On Thu, 2023-03-16 at 15:25 +0100, Hans de Goede wrote:
> Hi,
> 
> On 3/9/23 05:01, Dongliang Mu wrote:
> > The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
> > double free reported by Smatch") incorrectly handle the
> > deallocation of
> > res variable. As shown in the comment, intel_vsec_add_aux handles
> > all
> > the deallocation of res and feature_vsec_dev. Therefore, kfree(res)
> > can
> > still cause double free if intel_vsec_add_aux returns error.
> > 
> > Fix this by adjusting the error handling part in
> > tpmi_create_device,
> > following the function intel_vsec_add_dev.
> > 
> > Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free
> > reported by Smatch")
> > Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> 
> IIRC then after this v2 was posted I still saw some comments on the
> original v1 which was not posted on the list. Without the v1 comments
> being on the list and this archived, I have lost track of what the
> status of these patches is.
> 
> Srinivas, can you let me know if I should merge these, or if more
> changes are necessary ?
> 
> From the off-list discussion of v1 I got the impression more changes
> are necessary, but I'm not sure.

I was looking for changes submitted by the following patch
"
[PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak
in intel_vsec_add_aux
"

Since I was not copied on this, I was unaware. So I was requesting this
change.

Thanks,
Srinivas

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> > ---
> >  drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
> >  1 file changed, 4 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/tpmi.c
> > b/drivers/platform/x86/intel/tpmi.c
> > index c999732b0f1e..882fe5e4763f 100644
> > --- a/drivers/platform/x86/intel/tpmi.c
> > +++ b/drivers/platform/x86/intel/tpmi.c
> > @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
> > intel_tpmi_info *tpmi_info,
> >  
> >         feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev),
> > GFP_KERNEL);
> >         if (!feature_vsec_dev) {
> > -               ret = -ENOMEM;
> > -               goto free_res;
> > +               kfree(res);
> > +               return -ENOMEM;
> >         }
> >  
> >         snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-
> > %s", name);
> > @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
> > intel_tpmi_info *tpmi_info,
> >          * feature_vsec_dev memory is also freed as part of device
> >          * delete.
> >          */
> > -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
> > >auxdev.dev,
> > -                                feature_vsec_dev,
> > feature_id_name);
> > -       if (ret)
> > -               goto free_res;
> > -
> > -       return 0;
> > -
> > -free_res:
> > -       kfree(res);
> > -
> > -       return ret;
> > +       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
> > >auxdev.dev,
> > +                                 feature_vsec_dev,
> > feature_id_name);
> >  }
> >  
> >  static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)
> 

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Dongliang Mu]

On 2023/3/17 02:18, srinivas pandruvada wrote:
> Hi Hans,
>
> On Thu, 2023-03-16 at 15:25 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 3/9/23 05:01, Dongliang Mu wrote:
>>> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
>>> double free reported by Smatch") incorrectly handle the
>>> deallocation of
>>> res variable. As shown in the comment, intel_vsec_add_aux handles
>>> all
>>> the deallocation of res and feature_vsec_dev. Therefore, kfree(res)
>>> can
>>> still cause double free if intel_vsec_add_aux returns error.
>>>
>>> Fix this by adjusting the error handling part in
>>> tpmi_create_device,
>>> following the function intel_vsec_add_dev.
>>>
>>> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free
>>> reported by Smatch")
>>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
>> IIRC then after this v2 was posted I still saw some comments on the
>> original v1 which was not posted on the list. Without the v1 comments
>> being on the list and this archived, I have lost track of what the
>> status of these patches is.
>>
>> Srinivas, can you let me know if I should merge these, or if more
>> changes are necessary ?
>>
>>  From the off-list discussion of v1 I got the impression more changes
>> are necessary, but I'm not sure.
> I was looking for changes submitted by the following patch
> "
> [PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak
> in intel_vsec_add_aux
> "
>
> Since I was not copied on this, I was unaware. So I was requesting this
> change.
>
> Thanks,
> Srinivas

Hi Srinivas and Hans,

How about folding these three patches into one patch and resend a v3 patch?

This will get all people together and avoid the previous embarrassing 
sitation.

Dongliang Mu

>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>> ---
>>>   drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>>>   1 file changed, 4 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/intel/tpmi.c
>>> b/drivers/platform/x86/intel/tpmi.c
>>> index c999732b0f1e..882fe5e4763f 100644
>>> --- a/drivers/platform/x86/intel/tpmi.c
>>> +++ b/drivers/platform/x86/intel/tpmi.c
>>> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
>>> intel_tpmi_info *tpmi_info,
>>>   
>>>          feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev),
>>> GFP_KERNEL);
>>>          if (!feature_vsec_dev) {
>>> -               ret = -ENOMEM;
>>> -               goto free_res;
>>> +               kfree(res);
>>> +               return -ENOMEM;
>>>          }
>>>   
>>>          snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-
>>> %s", name);
>>> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
>>> intel_tpmi_info *tpmi_info,
>>>           * feature_vsec_dev memory is also freed as part of device
>>>           * delete.
>>>           */
>>> -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>> auxdev.dev,
>>> -                                feature_vsec_dev,
>>> feature_id_name);
>>> -       if (ret)
>>> -               goto free_res;
>>> -
>>> -       return 0;
>>> -
>>> -free_res:
>>> -       kfree(res);
>>> -
>>> -       return ret;
>>> +       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>> auxdev.dev,
>>> +                                 feature_vsec_dev,
>>> feature_id_name);
>>>   }
>>>   
>>>   static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Hans de Goede]

Hi,

On 3/17/23 02:28, Dongliang Mu wrote:
> 
> On 2023/3/17 02:18, srinivas pandruvada wrote:
>> Hi Hans,
>>
>> On Thu, 2023-03-16 at 15:25 +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 3/9/23 05:01, Dongliang Mu wrote:
>>>> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
>>>> double free reported by Smatch") incorrectly handle the
>>>> deallocation of
>>>> res variable. As shown in the comment, intel_vsec_add_aux handles
>>>> all
>>>> the deallocation of res and feature_vsec_dev. Therefore, kfree(res)
>>>> can
>>>> still cause double free if intel_vsec_add_aux returns error.
>>>>
>>>> Fix this by adjusting the error handling part in
>>>> tpmi_create_device,
>>>> following the function intel_vsec_add_dev.
>>>>
>>>> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free
>>>> reported by Smatch")
>>>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
>> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>
>>> IIRC then after this v2 was posted I still saw some comments on the
>>> original v1 which was not posted on the list. Without the v1 comments
>>> being on the list and this archived, I have lost track of what the
>>> status of these patches is.
>>>
>>> Srinivas, can you let me know if I should merge these, or if more
>>> changes are necessary ?
>>>
>>>  From the off-list discussion of v1 I got the impression more changes
>>> are necessary, but I'm not sure.
>> I was looking for changes submitted by the following patch
>> "
>> [PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak
>> in intel_vsec_add_aux
>> "
>>
>> Since I was not copied on this, I was unaware. So I was requesting this
>> change.
>>
>> Thanks,
>> Srinivas
> 
> Hi Srinivas and Hans,
> 
> How about folding these three patches into one patch and resend a v3 patch?
> 
> This will get all people together and avoid the previous embarrassing sitation.

If I understand things correctly then patch 1/3 needs 3/3 to function correctly, right ?

I would not fold them together, smaller patches are easier to review / understand, but maybe change the order and put patch 3/3 first? (so make it 1/3) ?

I can even do that when applying if you agree that is the better order.

Regards,

Hans



>>>> ---
>>>>   drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>>>>   1 file changed, 4 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/intel/tpmi.c
>>>> b/drivers/platform/x86/intel/tpmi.c
>>>> index c999732b0f1e..882fe5e4763f 100644
>>>> --- a/drivers/platform/x86/intel/tpmi.c
>>>> +++ b/drivers/platform/x86/intel/tpmi.c
>>>> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
>>>> intel_tpmi_info *tpmi_info,
>>>>            feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev),
>>>> GFP_KERNEL);
>>>>          if (!feature_vsec_dev) {
>>>> -               ret = -ENOMEM;
>>>> -               goto free_res;
>>>> +               kfree(res);
>>>> +               return -ENOMEM;
>>>>          }
>>>>            snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-
>>>> %s", name);
>>>> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
>>>> intel_tpmi_info *tpmi_info,
>>>>           * feature_vsec_dev memory is also freed as part of device
>>>>           * delete.
>>>>           */
>>>> -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>>> auxdev.dev,
>>>> -                                feature_vsec_dev,
>>>> feature_id_name);
>>>> -       if (ret)
>>>> -               goto free_res;
>>>> -
>>>> -       return 0;
>>>> -
>>>> -free_res:
>>>> -       kfree(res);
>>>> -
>>>> -       return ret;
>>>> +       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>>> auxdev.dev,
>>>> +                                 feature_vsec_dev,
>>>> feature_id_name);
>>>>   }
>>>>     static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[srinivas pandruvada]

Hi,

...
...

> > 

> > Hi Srinivas and Hans,
> > 
> > How about folding these three patches into one patch and resend a
> > v3 patch?
> > 
> > This will get all people together and avoid the previous
> > embarrassing sitation.
> 
> If I understand things correctly then patch 1/3 needs 3/3 to function
> correctly, right ?
> 
> I would not fold them together, smaller patches are easier to review
> / understand, but maybe change the order and put patch 3/3 first? (so
> make it 1/3) ?

That should be correct order. The patch 3/3 should be the first.

> 
> I can even do that when applying if you agree that is the better
> order.
Agree.

Thanks,
Srinivas

> 
> Regards,
> 
> Hans
> 
> 
> 
> > > > > ---
> > > > >   drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
> > > > >   1 file changed, 4 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/platform/x86/intel/tpmi.c
> > > > > b/drivers/platform/x86/intel/tpmi.c
> > > > > index c999732b0f1e..882fe5e4763f 100644
> > > > > --- a/drivers/platform/x86/intel/tpmi.c
> > > > > +++ b/drivers/platform/x86/intel/tpmi.c
> > > > > @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
> > > > > intel_tpmi_info *tpmi_info,
> > > > >            feature_vsec_dev =
> > > > > kzalloc(sizeof(*feature_vsec_dev),
> > > > > GFP_KERNEL);
> > > > >          if (!feature_vsec_dev) {
> > > > > -               ret = -ENOMEM;
> > > > > -               goto free_res;
> > > > > +               kfree(res);
> > > > > +               return -ENOMEM;
> > > > >          }
> > > > >            snprintf(feature_id_name, sizeof(feature_id_name),
> > > > > "tpmi-
> > > > > %s", name);
> > > > > @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
> > > > > intel_tpmi_info *tpmi_info,
> > > > >           * feature_vsec_dev memory is also freed as part of
> > > > > device
> > > > >           * delete.
> > > > >           */
> > > > > -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
> > > > > > auxdev.dev,
> > > > > -                                feature_vsec_dev,
> > > > > feature_id_name);
> > > > > -       if (ret)
> > > > > -               goto free_res;
> > > > > -
> > > > > -       return 0;
> > > > > -
> > > > > -free_res:
> > > > > -       kfree(res);
> > > > > -
> > > > > -       return ret;
> > > > > +       return intel_vsec_add_aux(vsec_dev->pcidev,
> > > > > &vsec_dev-
> > > > > > auxdev.dev,
> > > > > +                                 feature_vsec_dev,
> > > > > feature_id_name);
> > > > >   }
> > > > >     static int tpmi_create_devices(struct intel_tpmi_info
> > > > > *tpmi_info)
> 

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[srinivas pandruvada]

Hi Dongliang,

> 
...
...


> Hi Srinivas and Hans,
> 
> How about folding these three patches into one patch and resend a v3
> patch?
> 
> This will get all people together and avoid the previous embarrassing
> sitation.

This is NOT an embarrassing situation.
Thanks for finding and fixing the issue.

Thanks,
Srinivas


> 
> Dongliang Mu
> 
> > 
> > > Regards,
> > > 
> > > Hans
> > > 
> > > 
> > > 
> > > 
> > > > ---
> > > >   drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
> > > >   1 file changed, 4 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/intel/tpmi.c
> > > > b/drivers/platform/x86/intel/tpmi.c
> > > > index c999732b0f1e..882fe5e4763f 100644
> > > > --- a/drivers/platform/x86/intel/tpmi.c
> > > > +++ b/drivers/platform/x86/intel/tpmi.c
> > > > @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
> > > > intel_tpmi_info *tpmi_info,
> > > >   
> > > >          feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev),
> > > > GFP_KERNEL);
> > > >          if (!feature_vsec_dev) {
> > > > -               ret = -ENOMEM;
> > > > -               goto free_res;
> > > > +               kfree(res);
> > > > +               return -ENOMEM;
> > > >          }
> > > >   
> > > >          snprintf(feature_id_name, sizeof(feature_id_name),
> > > > "tpmi-
> > > > %s", name);
> > > > @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
> > > > intel_tpmi_info *tpmi_info,
> > > >           * feature_vsec_dev memory is also freed as part of
> > > > device
> > > >           * delete.
> > > >           */
> > > > -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
> > > > > auxdev.dev,
> > > > -                                feature_vsec_dev,
> > > > feature_id_name);
> > > > -       if (ret)
> > > > -               goto free_res;
> > > > -
> > > > -       return 0;
> > > > -
> > > > -free_res:
> > > > -       kfree(res);
> > > > -
> > > > -       return ret;
> > > > +       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
> > > > > auxdev.dev,
> > > > +                                 feature_vsec_dev,
> > > > feature_id_name);
> > > >   }
> > > >   
> > > >   static int tpmi_create_devices(struct intel_tpmi_info
> > > > *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Dongliang Mu]

On 2023/3/17 18:23, srinivas pandruvada wrote:
> Hi,
>
> ...
> ...
>
>>> Hi Srinivas and Hans,
>>>
>>> How about folding these three patches into one patch and resend a
>>> v3 patch?
>>>
>>> This will get all people together and avoid the previous
>>> embarrassing sitation.
>> If I understand things correctly then patch 1/3 needs 3/3 to function
>> correctly, right ?
>>
>> I would not fold them together, smaller patches are easier to review
>> / understand, but maybe change the order and put patch 3/3 first? (so
>> make it 1/3) ?
> That should be correct order. The patch 3/3 should be the first.

Oh, yeah. The memory leak fix 3/3 should be the first. This is more 
reasonable.

BTW, I separated this memory leak fix due to that the kernel mainline is 
still vulnerable to this memory leak problem.

>
>> I can even do that when applying if you agree that is the better
>> order.
> Agree.
>
> Thanks,
> Srinivas
>
>> Regards,
>>
>> Hans
>>
>>
>>
>>>>>> ---
>>>>>>    drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>>>>>>    1 file changed, 4 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/platform/x86/intel/tpmi.c
>>>>>> b/drivers/platform/x86/intel/tpmi.c
>>>>>> index c999732b0f1e..882fe5e4763f 100644
>>>>>> --- a/drivers/platform/x86/intel/tpmi.c
>>>>>> +++ b/drivers/platform/x86/intel/tpmi.c
>>>>>> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
>>>>>> intel_tpmi_info *tpmi_info,
>>>>>>             feature_vsec_dev =
>>>>>> kzalloc(sizeof(*feature_vsec_dev),
>>>>>> GFP_KERNEL);
>>>>>>           if (!feature_vsec_dev) {
>>>>>> -               ret = -ENOMEM;
>>>>>> -               goto free_res;
>>>>>> +               kfree(res);
>>>>>> +               return -ENOMEM;
>>>>>>           }
>>>>>>             snprintf(feature_id_name, sizeof(feature_id_name),
>>>>>> "tpmi-
>>>>>> %s", name);
>>>>>> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
>>>>>> intel_tpmi_info *tpmi_info,
>>>>>>            * feature_vsec_dev memory is also freed as part of
>>>>>> device
>>>>>>            * delete.
>>>>>>            */
>>>>>> -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>>>>> auxdev.dev,
>>>>>> -                                feature_vsec_dev,
>>>>>> feature_id_name);
>>>>>> -       if (ret)
>>>>>> -               goto free_res;
>>>>>> -
>>>>>> -       return 0;
>>>>>> -
>>>>>> -free_res:
>>>>>> -       kfree(res);
>>>>>> -
>>>>>> -       return ret;
>>>>>> +       return intel_vsec_add_aux(vsec_dev->pcidev,
>>>>>> &vsec_dev-
>>>>>>> auxdev.dev,
>>>>>> +                                 feature_vsec_dev,
>>>>>> feature_id_name);
>>>>>>    }
>>>>>>      static int tpmi_create_devices(struct intel_tpmi_info
>>>>>> *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Dongliang Mu]

On 2023/3/17 18:27, srinivas pandruvada wrote:
> Hi Dongliang,
>
> ...
> ...
>
>
>> Hi Srinivas and Hans,
>>
>> How about folding these three patches into one patch and resend a v3
>> patch?
>>
>> This will get all people together and avoid the previous embarrassing
>> sitation.
> This is NOT an embarrassing situation.
> Thanks for finding and fixing the issue.
>
> Thanks,
> Srinivas
>
Hi Srinivas,

Any conclusion about this patch set?

>> Dongliang Mu
>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>>
>>>>
>>>>
>>>>> ---
>>>>>    drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>>>>>    1 file changed, 4 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/platform/x86/intel/tpmi.c
>>>>> b/drivers/platform/x86/intel/tpmi.c
>>>>> index c999732b0f1e..882fe5e4763f 100644
>>>>> --- a/drivers/platform/x86/intel/tpmi.c
>>>>> +++ b/drivers/platform/x86/intel/tpmi.c
>>>>> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
>>>>> intel_tpmi_info *tpmi_info,
>>>>>    
>>>>>           feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev),
>>>>> GFP_KERNEL);
>>>>>           if (!feature_vsec_dev) {
>>>>> -               ret = -ENOMEM;
>>>>> -               goto free_res;
>>>>> +               kfree(res);
>>>>> +               return -ENOMEM;
>>>>>           }
>>>>>    
>>>>>           snprintf(feature_id_name, sizeof(feature_id_name),
>>>>> "tpmi-
>>>>> %s", name);
>>>>> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
>>>>> intel_tpmi_info *tpmi_info,
>>>>>            * feature_vsec_dev memory is also freed as part of
>>>>> device
>>>>>            * delete.
>>>>>            */
>>>>> -       ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>>>> auxdev.dev,
>>>>> -                                feature_vsec_dev,
>>>>> feature_id_name);
>>>>> -       if (ret)
>>>>> -               goto free_res;
>>>>> -
>>>>> -       return 0;
>>>>> -
>>>>> -free_res:
>>>>> -       kfree(res);
>>>>> -
>>>>> -       return ret;
>>>>> +       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev-
>>>>>> auxdev.dev,
>>>>> +                                 feature_vsec_dev,
>>>>> feature_id_name);
>>>>>    }
>>>>>    
>>>>>    static int tpmi_create_devices(struct intel_tpmi_info
>>>>> *tpmi_info)

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[srinivas pandruvada]

On Mon, 2023-03-20 at 10:43 +0800, Dongliang Mu wrote:
> 
> On 2023/3/17 18:27, srinivas pandruvada wrote:
> > Hi Dongliang,
> > 
> > ...
> > ...
> > 
> > 
> > > Hi Srinivas and Hans,
> > > 
> > > How about folding these three patches into one patch and resend a
> > > v3
> > > patch?
> > > 
> > > This will get all people together and avoid the previous
> > > embarrassing
> > > sitation.
> > This is NOT an embarrassing situation.
> > Thanks for finding and fixing the issue.
> > 
> > Thanks,
> > Srinivas
> > 
> Hi Srinivas,
> 
> Any conclusion about this patch set?

Hans can reorder patches as he suggested and apply.

Thanks,
Srinivas

> 
> > > Dongliang Mu
> > > 
> > > > > Regards,
> > > > > 
> > > > > Hans
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > ---
> > > > > >    drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
> > > > > >    1 file changed, 4 insertions(+), 13 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/platform/x86/intel/tpmi.c
> > > > > > b/drivers/platform/x86/intel/tpmi.c
> > > > > > index c999732b0f1e..882fe5e4763f 100644
> > > > > > --- a/drivers/platform/x86/intel/tpmi.c
> > > > > > +++ b/drivers/platform/x86/intel/tpmi.c
> > > > > > @@ -215,8 +215,8 @@ static int tpmi_create_device(struct
> > > > > > intel_tpmi_info *tpmi_info,
> > > > > >    
> > > > > >           feature_vsec_dev =
> > > > > > kzalloc(sizeof(*feature_vsec_dev),
> > > > > > GFP_KERNEL);
> > > > > >           if (!feature_vsec_dev) {
> > > > > > -               ret = -ENOMEM;
> > > > > > -               goto free_res;
> > > > > > +               kfree(res);
> > > > > > +               return -ENOMEM;
> > > > > >           }
> > > > > >    
> > > > > >           snprintf(feature_id_name,
> > > > > > sizeof(feature_id_name),
> > > > > > "tpmi-
> > > > > > %s", name);
> > > > > > @@ -242,17 +242,8 @@ static int tpmi_create_device(struct
> > > > > > intel_tpmi_info *tpmi_info,
> > > > > >            * feature_vsec_dev memory is also freed as part
> > > > > > of
> > > > > > device
> > > > > >            * delete.
> > > > > >            */
> > > > > > -       ret = intel_vsec_add_aux(vsec_dev->pcidev,
> > > > > > &vsec_dev-
> > > > > > > auxdev.dev,
> > > > > > -                                feature_vsec_dev,
> > > > > > feature_id_name);
> > > > > > -       if (ret)
> > > > > > -               goto free_res;
> > > > > > -
> > > > > > -       return 0;
> > > > > > -
> > > > > > -free_res:
> > > > > > -       kfree(res);
> > > > > > -
> > > > > > -       return ret;
> > > > > > +       return intel_vsec_add_aux(vsec_dev->pcidev,
> > > > > > &vsec_dev-
> > > > > > > auxdev.dev,
> > > > > > +                                 feature_vsec_dev,
> > > > > > feature_id_name);
> > > > > >    }
> > > > > >    
> > > > > >    static int tpmi_create_devices(struct intel_tpmi_info
> > > > > > *tpmi_info)

Re: [PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak in intel_vsec_add_aux

[Hans de Goede]

Hi,

On 3/9/23 05:01, Dongliang Mu wrote:
> The first error handling code in intel_vsec_add_aux misses the
> deallocation of intel_vsec_dev->resource.
> 
> Fix this by adding kfree(intel_vsec_dev->resource) in the error handling
> code.
> 
> Reviewed-by: David E. Box <david.e.box@linux.intel.com>
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

Thank you for your patch, I've applied this patch (as first patch
in the series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
>  drivers/platform/x86/intel/vsec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index 13decf36c6de..2311c16cb975 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -154,6 +154,7 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>  	ret = ida_alloc(intel_vsec_dev->ida, GFP_KERNEL);
>  	mutex_unlock(&vsec_ida_lock);
>  	if (ret < 0) {
> +		kfree(intel_vsec_dev->resource);
>  		kfree(intel_vsec_dev);
>  		return ret;
>  	}

Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device()

[Hans de Goede]

Hi,

On 3/9/23 05:01, Dongliang Mu wrote:
> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix
> double free reported by Smatch") incorrectly handle the deallocation of
> res variable. As shown in the comment, intel_vsec_add_aux handles all
> the deallocation of res and feature_vsec_dev. Therefore, kfree(res) can
> still cause double free if intel_vsec_add_aux returns error.
> 
> Fix this by adjusting the error handling part in tpmi_create_device,
> following the function intel_vsec_add_dev.
> 
> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free reported by Smatch")
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

Note this patch causes the following compiler warnings:

  CC [M]  drivers/platform/x86/intel/tpmi.o
drivers/platform/x86/intel/tpmi.c: In function ‘tpmi_create_device’:
drivers/platform/x86/intel/tpmi.c:206:13: warning: unused variable ‘ret’ [-Wunused-variable]
  206 |         int ret, i;
      |             ^~~

I have fixed this and squashed the fix into the patch while applying it,
so there is no need to send a new version:

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
>  drivers/platform/x86/intel/tpmi.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
> index c999732b0f1e..882fe5e4763f 100644
> --- a/drivers/platform/x86/intel/tpmi.c
> +++ b/drivers/platform/x86/intel/tpmi.c
> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>  
>  	feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev), GFP_KERNEL);
>  	if (!feature_vsec_dev) {
> -		ret = -ENOMEM;
> -		goto free_res;
> +		kfree(res);
> +		return -ENOMEM;
>  	}
>  
>  	snprintf(feature_id_name, sizeof(feature_id_name), "tpmi-%s", name);
> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>  	 * feature_vsec_dev memory is also freed as part of device
>  	 * delete.
>  	 */
> -	ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
> -				 feature_vsec_dev, feature_id_name);
> -	if (ret)
> -		goto free_res;
> -
> -	return 0;
> -
> -free_res:
> -	kfree(res);
> -
> -	return ret;
> +	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
> +				  feature_vsec_dev, feature_id_name);
>  }
>  
>  static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)

Re: [PATCH linux-next v2 2/3] drivers/platform/x86/intel/tpmi: revise the comment of intel_vsec_add_aux

[Hans de Goede]

Hi,

On 3/9/23 05:01, Dongliang Mu wrote:
> intel_vsec_add_aux() is resource managed including res and
> feature_vsec_dev memory.
> 
> Fix this by revising the comment of intel_vsec_add_aux since res variable
> will also be freed in the intel_vsec_add_aux.
> 
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
>  drivers/platform/x86/intel/tpmi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
> index 882fe5e4763f..036d0e0dba19 100644
> --- a/drivers/platform/x86/intel/tpmi.c
> +++ b/drivers/platform/x86/intel/tpmi.c
> @@ -239,8 +239,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>  	/*
>  	 * intel_vsec_add_aux() is resource managed, no explicit
>  	 * delete is required on error or on module unload.
> -	 * feature_vsec_dev memory is also freed as part of device
> -	 * delete.
> +	 * feature_vsec_dev and res memory are also freed as part of
> +	 * device deletion.
>  	 */
>  	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
>  				  feature_vsec_dev, feature_id_name);

Re: [PATCH linux-next v2 2/3] drivers/platform/x86/intel/tpmi: revise the comment of intel_vsec_add_aux

[Dongliang Mu]

On 2023/3/20 18:33, Hans de Goede wrote:
> Hi,
>
> On 3/9/23 05:01, Dongliang Mu wrote:
>> intel_vsec_add_aux() is resource managed including res and
>> feature_vsec_dev memory.
>>
>> Fix this by revising the comment of intel_vsec_add_aux since res variable
>> will also be freed in the intel_vsec_add_aux.
>>
>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> Regards,
>
> Hans

Thanks, Hans.

Shall I send the patch related to memory leak to the mainline? The 
mainline is also prone to this issue?

>
>
>
>> ---
>>   drivers/platform/x86/intel/tpmi.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
>> index 882fe5e4763f..036d0e0dba19 100644
>> --- a/drivers/platform/x86/intel/tpmi.c
>> +++ b/drivers/platform/x86/intel/tpmi.c
>> @@ -239,8 +239,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>>   	/*
>>   	 * intel_vsec_add_aux() is resource managed, no explicit
>>   	 * delete is required on error or on module unload.
>> -	 * feature_vsec_dev memory is also freed as part of device
>> -	 * delete.
>> +	 * feature_vsec_dev and res memory are also freed as part of
>> +	 * device deletion.
>>   	 */
>>   	return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
>>   				  feature_vsec_dev, feature_id_name);

Re: [PATCH linux-next v2 2/3] drivers/platform/x86/intel/tpmi: revise the comment of intel_vsec_add_aux

[Hans de Goede]

Hi,

On 3/20/23 11:57, Dongliang Mu wrote:
> 
> On 2023/3/20 18:33, Hans de Goede wrote:
>> Hi,
>>
>> On 3/9/23 05:01, Dongliang Mu wrote:
>>> intel_vsec_add_aux() is resource managed including res and
>>> feature_vsec_dev memory.
>>>
>>> Fix this by revising the comment of intel_vsec_add_aux since res variable
>>> will also be freed in the intel_vsec_add_aux.
>>>
>>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
>> Thank you for your patch, I've applied this patch to my review-hans
>> branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>>
>> Note it will show up in my review-hans branch once I've pushed my
>> local branch there, which might take a while.
>>
>> Once I've run some tests on this branch the patches there will be
>> added to the platform-drivers-x86/for-next branch and eventually
>> will be included in the pdx86 pull-request to Linus for the next
>> merge-window.
>>
>> Regards,
>>
>> Hans
> 
> Thanks, Hans.
> 
> Shall I send the patch related to memory leak to the mainline? The mainline is also prone to this issue?

There is no need for this, I have just moved these from my review-hans branch to the fixes
branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

So these will be included in my next fixes pull-req for mainline.

Regards,

Hans




>>> ---
>>>   drivers/platform/x86/intel/tpmi.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
>>> index 882fe5e4763f..036d0e0dba19 100644
>>> --- a/drivers/platform/x86/intel/tpmi.c
>>> +++ b/drivers/platform/x86/intel/tpmi.c
>>> @@ -239,8 +239,8 @@ static int tpmi_create_device(struct intel_tpmi_info *tpmi_info,
>>>       /*
>>>        * intel_vsec_add_aux() is resource managed, no explicit
>>>        * delete is required on error or on module unload.
>>> -     * feature_vsec_dev memory is also freed as part of device
>>> -     * delete.
>>> +     * feature_vsec_dev and res memory are also freed as part of
>>> +     * device deletion.
>>>        */
>>>       return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev->auxdev.dev,
>>>                     feature_vsec_dev, feature_id_name);
>