Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] nvdimm: memory leak on device_add() failure in nd_async_device_register() --
commit 3bb889471cc9122db778cd1b964d9131ca101b98 Author: Wentao Liang <[email protected]> nvdimm/bus: Fix dev double put in nd_async_device_register() device_add() takes and drops its own reference on every return path, so the async worker only owns the reference that __nd_device_register() took with get_device(). The failure branch drops that same reference once more before the unconditional put_device(), which underflows the refcount and frees the device while it is still referenced by its creator. > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > index 45b7d756e39a0..a1f33a03aba81 100644 > --- a/drivers/nvdimm/bus.c > +++ b/drivers/nvdimm/bus.c > @@ -488,10 +488,8 @@ static void nd_async_device_register(void *d, > async_cookie_t cookie) > struct device *dev = d; > struct device *parent = dev->parent; > > - if (device_add(dev) != 0) { > + if (device_add(dev) != 0) > dev_err(dev, "%s: failed\n", __func__); > - put_device(dev); > - } [Severity: Medium] Will this lead to a memory leak of the device if device_add() fails? Devices initialized with device_initialize() start with a refcount of 1. In __nd_device_register(), a second reference is taken before queuing the async worker. If device_add() fails here, it does not consume a reference. By removing the extra put_device() from the failure path, the async worker only drops the second reference, leaving the device with a refcount of 1. Because device_add() failed, the device isn't added to the parent's klist_children, so it won't be unregistered when the parent is removed. Since the creator simply nullifies its pointer without dropping the final reference, does this result in a permanent leak of the device and its container structures? > put_device(dev); > if (parent) > put_device(parent); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
