Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
On Thu, Jan 7, 2021 at 8:36 PM 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
> Cc:
> ---
> mm/hugetlb.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Muchun Song
>
> 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
>
Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
On 2021/1/8 7:15, Andrew Morton wrote:
> On Thu, 7 Jan 2021 11:59:38 -0800 Mike Kravetz
> 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
>>> Cc:
>>> ---
>>> 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
>>
>> 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!
Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
On Thu, 7 Jan 2021 11:59:38 -0800 Mike Kravetz 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
> > Cc:
> > ---
> > 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
>
> 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.
Re: [PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
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
> Cc:
> ---
> 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
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
[PATCH] mm/hugetlb: Fix potential double free in hugetlb_register_node() error path
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
Cc:
---
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
