Re: [PATCH] dmaengine: idxd: fix double free in idxd_alloc() error path

[Guangshuo Li]

Hi Vinicius,

Thanks for reviewing  — the feedback is helpful.

I'm working on top of v6.19-rc8-214-ge7aa57247700.

Regarding the concern about put_device(conf_dev) triggering
idxd_conf_device_release() and hitting a NULL idxd->wq in
destroy_workqueue():

idxd_conf_device_release() does not call destroy_workqueue(). That
call lives in idxd_cleanup_internals(), which is a separate code path.
The actual release callback is:

static void idxd_conf_device_release(struct device *dev)
{
    struct idxd_device *idxd = confdev_to_idxd(dev);

    kfree(idxd->groups);
    bitmap_free(idxd->wq_enable_map);
    kfree(idxd->wqs);
    kfree(idxd->engines);
    kfree(idxd->evl);
    kmem_cache_destroy(idxd->evl_cache);
    ida_free(&idxd_ida, idxd->id);
    bitmap_free(idxd->opcap_bmap);
    kfree(idxd);
}

At the err_name point in idxd_alloc(), idxd was allocated with
kzalloc_node(), so all uninitialized fields are zero/NULL. Every
function in the release callback handles NULL safely:

kfree(NULL) — safe
bitmap_free(NULL) — safe (wraps kfree)
kmem_cache_destroy(NULL) — safe (explicit NULL check at entry)
ida_free(&idxd_ida, idxd->id) — id is already allocated at this point
bitmap_free(idxd->opcap_bmap) — already allocated at this point
So relying on put_device() → idxd_conf_device_release() to clean up is
correct for this error path.

Regarding the other points:

I agree the patches should be sent as a numbered series.
For the put_device()-then-kfree() double-free pattern in
idxd_clean_wqs(), idxd_clean_engines(), idxd_clean_groups(), and
idxd_free(), I'll address those in the same series.
Will send a v2 series shortly.

Thanks,
Guangshuo