All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
@ 2021-01-07 12:32 Miaohe Lin
  2021-01-07 19:59 ` Mike Kravetz
  2021-01-09  4:36   ` Muchun Song
  0 siblings, 2 replies; 6+ messages in thread
From: Miaohe Lin @ 2021-01-07 12:32 UTC (permalink / raw)
  To: akpm, mike.kravetz; +Cc: linux-mm, linux-kernel, linmiaohe

In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
when failed to create sysfs group but forget to set hstate_kobjs to NULL.
Then in hugetlb_register_node() error path, we may free it again via
hugetlb_unregister_node().

Fixes: a3437870160c ("hugetlb: new sysfs interface")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Cc: <stable@vger.kernel.org>
---
 mm/hugetlb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e249bffa0e75..91a2a2025a2c 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2947,8 +2947,10 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent,
 		return -ENOMEM;
 
 	retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
-	if (retval)
+	if (retval) {
 		kobject_put(hstate_kobjs[hi]);
+		hstate_kobjs[hi] = NULL;
+	}
 
 	return retval;
 }
-- 
2.19.1


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

* Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
  2021-01-07 12:32 [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path Miaohe Lin
@ 2021-01-07 19:59 ` Mike Kravetz
  2021-01-07 23:15   ` Andrew Morton
  2021-01-09  4:36   ` Muchun Song
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Kravetz @ 2021-01-07 19:59 UTC (permalink / raw)
  To: Miaohe Lin, akpm; +Cc: linux-mm, linux-kernel

On 1/7/21 4:32 AM, Miaohe Lin wrote:
> In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
> when failed to create sysfs group but forget to set hstate_kobjs to NULL.
> Then in hugetlb_register_node() error path, we may free it again via
> hugetlb_unregister_node().
> 
> Fixes: a3437870160c ("hugetlb: new sysfs interface")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> Cc: <stable@vger.kernel.org>
> ---
>  mm/hugetlb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, this is a potential issue that should be fixed.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

This has been around for a long time (more than 12 years).  I suspect
nobody actually experienced this issue.  You just discovered via code
inspection.  Correct?
At one time cc stable would not be accepted for this type of issue,
not sure about today.
-- 
Mike Kravetz

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

* Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
  2021-01-07 19:59 ` Mike Kravetz
@ 2021-01-07 23:15   ` Andrew Morton
  2021-01-08  1:41     ` Miaohe Lin
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2021-01-07 23:15 UTC (permalink / raw)
  To: Mike Kravetz; +Cc: Miaohe Lin, linux-mm, linux-kernel

On Thu, 7 Jan 2021 11:59:38 -0800 Mike Kravetz <mike.kravetz@oracle.com> wrote:

> On 1/7/21 4:32 AM, Miaohe Lin wrote:
> > In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
> > when failed to create sysfs group but forget to set hstate_kobjs to NULL.
> > Then in hugetlb_register_node() error path, we may free it again via
> > hugetlb_unregister_node().
> > 
> > Fixes: a3437870160c ("hugetlb: new sysfs interface")
> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> >  mm/hugetlb.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Thanks, this is a potential issue that should be fixed.
> 
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> 
> This has been around for a long time (more than 12 years).  I suspect
> nobody actually experienced this issue.  You just discovered via code
> inspection.  Correct?
> At one time cc stable would not be accepted for this type of issue,
> not sure about today.

sysfs_create_group() will only fail if something is terribly messed up
- probably it has never happened to anyone.  I don't think the
cc:stable is justified here.


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

* Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
  2021-01-07 23:15   ` Andrew Morton
@ 2021-01-08  1:41     ` Miaohe Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Miaohe Lin @ 2021-01-08  1:41 UTC (permalink / raw)
  To: Andrew Morton, Mike Kravetz; +Cc: linux-mm, linux-kernel

On 2021/1/8 7:15, Andrew Morton wrote:
> On Thu, 7 Jan 2021 11:59:38 -0800 Mike Kravetz <mike.kravetz@oracle.com> wrote:
> 
>> On 1/7/21 4:32 AM, Miaohe Lin wrote:
>>> In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
>>> when failed to create sysfs group but forget to set hstate_kobjs to NULL.
>>> Then in hugetlb_register_node() error path, we may free it again via
>>> hugetlb_unregister_node().
>>>
>>> Fixes: a3437870160c ("hugetlb: new sysfs interface")
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>  mm/hugetlb.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> Thanks, this is a potential issue that should be fixed.
>>
>> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
>>
>> This has been around for a long time (more than 12 years).  I suspect
>> nobody actually experienced this issue.  You just discovered via code
>> inspection.  Correct?

Yes, I found this by code inspection.

>> At one time cc stable would not be accepted for this type of issue,
>> not sure about today.
> 
> sysfs_create_group() will only fail if something is terribly messed up
> - probably it has never happened to anyone.  I don't think the
> cc:stable is justified here.
> 
> .
> 

I would take care of more when cc stable. Many thanks for both of you!

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

* Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
  2021-01-07 12:32 [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path Miaohe Lin
@ 2021-01-09  4:36   ` Muchun Song
  2021-01-09  4:36   ` Muchun Song
  1 sibling, 0 replies; 6+ messages in thread
From: Muchun Song @ 2021-01-09  4:36 UTC (permalink / raw)
  To: Miaohe Lin; +Cc: Andrew Morton, mike.kravetz, linux-mm, linux-kernel

On Thu, Jan 7, 2021 at 8:36 PM Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
> when failed to create sysfs group but forget to set hstate_kobjs to NULL.
> Then in hugetlb_register_node() error path, we may free it again via
> hugetlb_unregister_node().
>
> Fixes: a3437870160c ("hugetlb: new sysfs interface")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> Cc: <stable@vger.kernel.org>
> ---
>  mm/hugetlb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Muchun Song <smuchun@gmail.com>

>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index e249bffa0e75..91a2a2025a2c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2947,8 +2947,10 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent,
>                 return -ENOMEM;
>
>         retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
> -       if (retval)
> +       if (retval) {
>                 kobject_put(hstate_kobjs[hi]);
> +               hstate_kobjs[hi] = NULL;
> +       }
>
>         return retval;
>  }
> --
> 2.19.1
>

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

* Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
@ 2021-01-09  4:36   ` Muchun Song
  0 siblings, 0 replies; 6+ messages in thread
From: Muchun Song @ 2021-01-09  4:36 UTC (permalink / raw)
  To: Miaohe Lin; +Cc: Andrew Morton, mike.kravetz, linux-mm, linux-kernel

On Thu, Jan 7, 2021 at 8:36 PM Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> In hugetlb_sysfs_add_hstate(), we would do kobject_put() on hstate_kobjs
> when failed to create sysfs group but forget to set hstate_kobjs to NULL.
> Then in hugetlb_register_node() error path, we may free it again via
> hugetlb_unregister_node().
>
> Fixes: a3437870160c ("hugetlb: new sysfs interface")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> Cc: <stable@vger.kernel.org>
> ---
>  mm/hugetlb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Muchun Song <smuchun@gmail.com>

>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index e249bffa0e75..91a2a2025a2c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2947,8 +2947,10 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent,
>                 return -ENOMEM;
>
>         retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
> -       if (retval)
> +       if (retval) {
>                 kobject_put(hstate_kobjs[hi]);
> +               hstate_kobjs[hi] = NULL;
> +       }
>
>         return retval;
>  }
> --
> 2.19.1
>


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

end of thread, other threads:[~2021-01-09  4:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07 12:32 [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path Miaohe Lin
2021-01-07 19:59 ` Mike Kravetz
2021-01-07 23:15   ` Andrew Morton
2021-01-08  1:41     ` Miaohe Lin
2021-01-09  4:36 ` Muchun Song
2021-01-09  4:36   ` Muchun Song

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.