[PATCH v2] mm: thp: Fix refcount leak in thpsize_create() error path

Guangshuo Li posted 1 patch 2 months ago
mm/huge_memory.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH v2] mm: thp: Fix refcount leak in thpsize_create() error path
Posted by Guangshuo Li 2 months ago
After kobject_init_and_add(), the lifetime of the embedded struct
kobject is expected to be managed through the kobject core reference
counting.

In thpsize_create(), if kobject_init_and_add() fails, thpsize is freed
directly with kfree() rather than releasing the kobject reference with
kobject_put(). This may leave the reference count of the embedded struct
kobject unbalanced, resulting in a refcount leak.

Fix this by using kobject_put(&thpsize->kobj) in the failure path and
letting thpsize_release() handle the final cleanup.

Fixes: 3485b88390b0 ("mm: thp: introduce multi-size THP sysfs interface")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Drop the incorrect UAF mention from the commit message
  - Clarify that the bug is an unbalanced kobject reference in the
  - kobject_init_and_add() failure path

 mm/huge_memory.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 40cf59301c21..c8ffa188a198 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -726,10 +726,8 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
 
 	ret = kobject_init_and_add(&thpsize->kobj, &thpsize_ktype, parent,
 				   "hugepages-%lukB", size);
-	if (ret) {
-		kfree(thpsize);
+	if (ret)
 		goto err;
-	}
 
 
 	ret = sysfs_add_group(&thpsize->kobj, &any_ctrl_attr_grp);
-- 
2.43.0
Re: [PATCH v2] mm: thp: Fix refcount leak in thpsize_create() error path
Posted by David Hildenbrand (Arm) 2 months ago
On 4/12/26 19:54, Guangshuo Li wrote:
> After kobject_init_and_add(), the lifetime of the embedded struct
> kobject is expected to be managed through the kobject core reference
> counting.
> 
> In thpsize_create(), if kobject_init_and_add() fails, thpsize is freed
> directly with kfree() rather than releasing the kobject reference with
> kobject_put(). This may leave the reference count of the embedded struct
> kobject unbalanced, resulting in a refcount leak.
> 
> Fix this by using kobject_put(&thpsize->kobj) in the failure path and
> letting thpsize_release() handle the final cleanup.

Fortunately, that's only called during init, and the stars would have to
align for this to ever trigger. I don't think this would ever trigger on
any reasonable system, really.

And if it triggers, other stuff would already be severely messed up.

> 
> Fixes: 3485b88390b0 ("mm: thp: introduce multi-size THP sysfs interface")
> Cc: stable@vger.kernel.org

Which also makes me wonder whether we even care about stable here.

> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
>   - Drop the incorrect UAF mention from the commit message
>   - Clarify that the bug is an unbalanced kobject reference in the
>   - kobject_init_and_add() failure path
> 
>  mm/huge_memory.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 40cf59301c21..c8ffa188a198 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -726,10 +726,8 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
>  
>  	ret = kobject_init_and_add(&thpsize->kobj, &thpsize_ktype, parent,
>  				   "hugepages-%lukB", size);
> -	if (ret) {
> -		kfree(thpsize);
> +	if (ret)
>  		goto err;
> -	}

kobject_init_and_add() indeed documents "If this function returns an
error, kobject_put() must be called".

As Andrew says, that's not what the "goto err" does.

-- 
Cheers,

David
Re: [PATCH v2] mm: thp: Fix refcount leak in thpsize_create() error path
Posted by Lance Yang 2 months ago

On 2026/4/13 19:39, David Hildenbrand (Arm) wrote:
[...]
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 40cf59301c21..c8ffa188a198 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -726,10 +726,8 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
>>   
>>   	ret = kobject_init_and_add(&thpsize->kobj, &thpsize_ktype, parent,
>>   				   "hugepages-%lukB", size);
>> -	if (ret) {
>> -		kfree(thpsize);
>> +	if (ret)
>>   		goto err;
>> -	}
> 
> kobject_init_and_add() indeed documents "If this function returns an
> error, kobject_put() must be called".
> 
> As Andrew says, that's not what the "goto err" does.

Right. v1[1] should do the trick: just jump to err_put

-	if (ret) {
-		kfree(thpsize);
-		goto err;
-	}
-
+	if (ret)
+		goto err_put;

Hmm... not sure why this changed to "goto err"

[1] 
https://lore.kernel.org/linux-mm/20260411062152.2092967-1-lgs201920130244@gmail.com/#t
Re: [PATCH v2] mm: thp: Fix refcount leak in thpsize_create() error path
Posted by Andrew Morton 2 months ago
On Mon, 13 Apr 2026 01:54:28 +0800 Guangshuo Li <lgs201920130244@gmail.com> wrote:

> After kobject_init_and_add(), the lifetime of the embedded struct
> kobject is expected to be managed through the kobject core reference
> counting.
> 
> In thpsize_create(), if kobject_init_and_add() fails, thpsize is freed
> directly with kfree() rather than releasing the kobject reference with
> kobject_put(). This may leave the reference count of the embedded struct
> kobject unbalanced, resulting in a refcount leak.
> 
> Fix this by using kobject_put(&thpsize->kobj) in the failure path and
> letting thpsize_release() handle the final cleanup.

OK...

> 
> ...
>
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -726,10 +726,8 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
>  
>  	ret = kobject_init_and_add(&thpsize->kobj, &thpsize_ktype, parent,
>  				   "hugepages-%lukB", size);
> -	if (ret) {
> -		kfree(thpsize);
> +	if (ret)
>  		goto err;

So this should be goto err_put?

> -	}
>  
>  
>  	ret = sysfs_add_group(&thpsize->kobj, &any_ctrl_attr_grp);
> -- 
> 2.43.0