drivers/md/md.c | 3 +++ 1 file changed, 3 insertions(+)
md_import_device() initializes rdev->kobj with kobject_init() before
checking the device size and loading the superblock.
When one of the later checks fails, the error path still frees rdev
directly with kfree(). This bypasses the kobject release path and leaves
the kobject reference unbalanced.
The issue was identified by a static analysis tool I developed and
confirmed by manual review.
After kobject_init(), release rdev through kobject_put() instead of
kfree().
Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to kobject_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
- note that the issue was identified by my static analysis tool
- and confirmed by manual review
drivers/md/md.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 6d73f6e196a9..4ce7512dc834 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3871,6 +3871,9 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
out_blkdev_put:
fput(rdev->bdev_file);
+ md_rdev_clear(rdev);
+ kobject_put(&rdev->kobj);
+ return ERR_PTR(err);
out_clear_rdev:
md_rdev_clear(rdev);
out_free_rdev:
--
2.43.0
Hi,
在 2026/4/13 22:17, Guangshuo Li 写道:
> md_import_device() initializes rdev->kobj with kobject_init() before
> checking the device size and loading the superblock.
>
> When one of the later checks fails, the error path still frees rdev
> directly with kfree(). This bypasses the kobject release path and leaves
> the kobject reference unbalanced.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
>
> After kobject_init(), release rdev through kobject_put() instead of
> kfree().
>
> Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to kobject_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
> - note that the issue was identified by my static analysis tool
> - and confirmed by manual review
>
> drivers/md/md.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6d73f6e196a9..4ce7512dc834 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3871,6 +3871,9 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> + md_rdev_clear(rdev);
> + kobject_put(&rdev->kobj);
> + return ERR_PTR(err);
I think it's cleaner to move kobject_init() after everything in rdev
is ready.
> out_clear_rdev:
> md_rdev_clear(rdev);
> out_free_rdev:
--
Thansk,
Kuai
On Mon 13 Apr 2026 at 22:17, Guangshuo Li
<lgs201920130244@gmail.com> wrote:
> md_import_device() initializes rdev->kobj with kobject_init()
> before
> checking the device size and loading the superblock.
>
> When one of the later checks fails, the error path still frees
> rdev
> directly with kfree(). This bypasses the kobject release path
> and leaves
> the kobject reference unbalanced.
>
> The issue was identified by a static analysis tool I developed
> and
> confirmed by manual review.
>
> After kobject_init(), release rdev through kobject_put() instead
> of
> kfree().
>
> Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to
> kobject_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
> - note that the issue was identified by my static analysis
> tool
> - and confirmed by manual review
>
> drivers/md/md.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6d73f6e196a9..4ce7512dc834 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3871,6 +3871,9 @@ static struct md_rdev
> *md_import_device(dev_t newdev, int super_format, int supe
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> + md_rdev_clear(rdev);
> + kobject_put(&rdev->kobj);
> + return ERR_PTR(err);
>
Why not just:
out_blkdev_put:
kobject_put(&rdev->kobj);
fput(rdev->bdev_file);
out_clear_rdev:
md_rdev_clear(rdev);
out_free_rdev:
kfree(rdev);
return ERR_PTR(err);
--
Su
Hi Su,
Thanks for reviewing.
On Tue, 14 Apr 2026 at 09:29, Su Yue <l@damenly.org> wrote:
> Why not just:
>
> out_blkdev_put:
> kobject_put(&rdev->kobj);
> fput(rdev->bdev_file);
> out_clear_rdev:
> md_rdev_clear(rdev);
> out_free_rdev:
> kfree(rdev);
> return ERR_PTR(err);
>
> --
> Su
I wonder if that ordering might cause a problem.
After kobject_init(&rdev->kobj, &rdev_ktype), kobject_put(&rdev->kobj)
may immediately drop the last reference and run the release callback
from rdev_ktype:
static const struct kobj_type rdev_ktype = {
.release = rdev_free,
.sysfs_ops = &rdev_sysfs_ops,
.default_groups = rdev_default_groups,
};
static void rdev_free(struct kobject *ko)
{
struct md_rdev *rdev = container_of(ko, struct md_rdev, kobj);
kfree(rdev);
}
So in:
out_blkdev_put:
kobject_put(&rdev->kobj);
fput(rdev->bdev_file);
it seems possible that kobject_put() would already free rdev via
rdev_free(), and then fput(rdev->bdev_file) would dereference rdev
after free.
That was why I changed it to:
out_blkdev_put:
fput(rdev->bdev_file);
md_rdev_clear(rdev);
kobject_put(&rdev->kobj);
return ERR_PTR(err);
so that the cleanup which still needs rdev is done before
kobject_put(), and this path returns directly instead of falling
through to the old kfree(rdev) path.
Please let me know if I overlooked something.
Thanks,
Guangshuo
On Tue 14 Apr 2026 at 19:32, Guangshuo Li
<lgs201920130244@gmail.com> wrote:
> Hi Su,
>
> Thanks for reviewing.
>
> On Tue, 14 Apr 2026 at 09:29, Su Yue <l@damenly.org> wrote:
>> Why not just:
>>
>> out_blkdev_put:
>> kobject_put(&rdev->kobj);
>> fput(rdev->bdev_file);
>> out_clear_rdev:
>> md_rdev_clear(rdev);
>> out_free_rdev:
>> kfree(rdev);
>> return ERR_PTR(err);
>>
>> --
>> Su
>
> I wonder if that ordering might cause a problem.
>
> After kobject_init(&rdev->kobj, &rdev_ktype),
> kobject_put(&rdev->kobj)
> may immediately drop the last reference and run the release
> callback
> from rdev_ktype:
>
> static const struct kobj_type rdev_ktype = {
> .release = rdev_free,
> .sysfs_ops = &rdev_sysfs_ops,
> .default_groups = rdev_default_groups,
> };
>
> static void rdev_free(struct kobject *ko)
> {
> struct md_rdev *rdev = container_of(ko, struct md_rdev,
> kobj);
> kfree(rdev);
> }
>
> So in:
>
> out_blkdev_put:
> kobject_put(&rdev->kobj);
> fput(rdev->bdev_file);
>
> it seems possible that kobject_put() would already free rdev via
> rdev_free(), and then fput(rdev->bdev_file) would dereference
> rdev
> after free.
>
> That was why I changed it to:
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> md_rdev_clear(rdev);
> kobject_put(&rdev->kobj);
> return ERR_PTR(err);
>
> so that the cleanup which still needs rdev is done before
> kobject_put(), and this path returns directly instead of falling
> through to the old kfree(rdev) path.
>
> Please let me know if I overlooked something.
>
Thanks for your detailed explanation. It's totally correct.
--
Su
> Thanks,
> Guangshuo
© 2016 - 2026 Red Hat, Inc.