Re: [PATCH] EDAC/device: Fix double free on sysfs registration failure

[Guangshuo Li]

Hi Borislav,

Thanks for the review.

On Thu, 13 Aug 2026 at 14:17, Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Tue, Jul 14, 2026 at 08:11:13PM +0800, Guangshuo Li wrote:
> > edac_device_alloc_ctl_info() allocates dev_ctl and passes it to
> > edac_device_register_sysfs_main_kobj() to initialize its embedded
> > kobject.
> >
> > If kobject_init_and_add() fails, the helper calls kobject_put(). The
> > final reference invokes edac_device_ctrl_master_release(), which calls
> > __edac_device_free_ctl_info() and frees dev_ctl for the first time.
>
> So far so good.
>
> > After the helper returns an error, edac_device_alloc_ctl_info() jumps
> > to its free label and calls __edac_device_free_ctl_info() on the same
> > object again, resulting in a double free.
>
> This is where you lost me.
>
> Because:
>
> /* Free the actual struct */
> static inline void __edac_device_free_ctl_info(struct edac_device_ctl_info *ci)
> {
>         if (ci) {
>         ^^^^^^^^^^
>
> Otherwise, we would've caught the double-free a bunch of times now.
>
> I do like the cleanup in edac_device_register_sysfs_main_kobj() though. This
>
>         dev_root = bus_get_dev_root(edac_subsys);
>         if (dev_root) {
>                 err = kobject_init_and_add(&edac_dev->kobj, &ktype_device_ctrl,
>
> is an antipattern and needs to go and I like how you're doing
>
>         if (!dev_root)
>
> so I'd take that cleanup gladly.
>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

The if (ci) only checks whether the pointer is NULL; kfree() does not
clear the caller's pointer. If kobject_put() drops the last reference,
edac_device_ctrl_master_release() frees edac_dev, so the following
module_put(edac_dev->owner) already accesses freed memory, and the
allocator may later free the same dev_ctl again.

Thanks,
Guangshuo