Message ID | 20210107123249.36964-1-linmiaohe@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | mm/hugetlb: Fix potential double free in hugetlb_register_node() error path | expand |
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.
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.
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!
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 >
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; }
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(-)