[PATCH] drbd: Fix local_cnt refcount leak on ascw allocation failure in _drbd_set_state

[PATCH] drbd: Fix local_cnt refcount leak on ascw allocation failure in _drbd_set_state

[Wentao Liang]

In _drbd_set_state(), when transitioning a device to D_FAILED or
D_DISKLESS, an extra reference on local_cnt is taken via
atomic_inc(&device->local_cnt) to prevent premature destruction of
the local disk. This reference is normally released by put_ldev()
in after_state_ch(), which is called asynchronously through the
after_state_chg_work (ascw) work item.

If the GFP_ATOMIC allocation of the ascw work item fails, the work
is never queued, after_state_ch() never runs, and the extra
local_cnt reference is permanently leaked. Additionally, the
state_change object allocated by remember_old_state() is also
leaked, along with the krefs it acquired on the resource,
connections, and devices.

Fix both leaks in the ascw allocation failure path:
 - Call put_ldev() to release the extra local_cnt reference when
   the transition matches the same conditions used for the
   atomic_inc.
 - Call forget_state_change() to free the state_change object and
   release the krefs it holds.

Cc: stable@vger.kernel.org
Fixes: d01801710265 ("drbd: Remove the terrible DEV hack")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/block/drbd/drbd_state.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index adcba7f1d8ea..68e273c6d5be 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1480,7 +1480,13 @@ _drbd_set_state(struct drbd_device *device, union drbd_state ns,
 		drbd_queue_work(&connection->sender_work,
 				&ascw->w);
 	} else {
-		drbd_err(device, "Could not kmalloc an ascw\n");
+		if ((os.disk != D_FAILED && ns.disk == D_FAILED) ||
+		    (os.disk != D_DISKLESS && ns.disk == D_DISKLESS))
+			put_ldev(device);
+
+		forget_state_change(state_change);
+		drbd_err(device, "Could not kmalloc an ascw, state change %p -> %p leaked\n",
+			 &os, &ns);
 	}
 
 	return rv;
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] drbd: Fix local_cnt refcount leak on ascw allocation failure in _drbd_set_state

[Christoph Böhmwalder]

On Thu, Jun 25, 2026 at 11:16:36PM +0800, Wentao Liang wrote:
>In _drbd_set_state(), when transitioning a device to D_FAILED or
>D_DISKLESS, an extra reference on local_cnt is taken via
>atomic_inc(&device->local_cnt) to prevent premature destruction of
>the local disk. This reference is normally released by put_ldev()
>in after_state_ch(), which is called asynchronously through the
>after_state_chg_work (ascw) work item.
>
>If the GFP_ATOMIC allocation of the ascw work item fails, the work
>is never queued, after_state_ch() never runs, and the extra
>local_cnt reference is permanently leaked. Additionally, the
>state_change object allocated by remember_old_state() is also
>leaked, along with the krefs it acquired on the resource,
>connections, and devices.
>
>Fix both leaks in the ascw allocation failure path:
> - Call put_ldev() to release the extra local_cnt reference when
>   the transition matches the same conditions used for the
>   atomic_inc.
> - Call forget_state_change() to free the state_change object and
>   release the krefs it holds.
>
>Cc: stable@vger.kernel.org
>Fixes: d01801710265 ("drbd: Remove the terrible DEV hack")
>Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
>---
> drivers/block/drbd/drbd_state.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
>index adcba7f1d8ea..68e273c6d5be 100644
>--- a/drivers/block/drbd/drbd_state.c
>+++ b/drivers/block/drbd/drbd_state.c
>@@ -1480,7 +1480,13 @@ _drbd_set_state(struct drbd_device *device, union drbd_state ns,
> 		drbd_queue_work(&connection->sender_work,
> 				&ascw->w);
> 	} else {
>-		drbd_err(device, "Could not kmalloc an ascw\n");
>+		if ((os.disk != D_FAILED && ns.disk == D_FAILED) ||
>+		    (os.disk != D_DISKLESS && ns.disk == D_DISKLESS))
>+			put_ldev(device);

Thanks for the patch. The logic itself looks correct to me.

>+
>+		forget_state_change(state_change);
>+		drbd_err(device, "Could not kmalloc an ascw, state change %p -> %p leaked\n",
>+			 &os, &ns);

However, this error message is nonsensical. If anything, we should print 
some halfway human-readable identifier for the state values here, not 
the pointer.

Also, the state change is precisely *not* leaked at the point this
message triggers, since we free it here. What actually gets lost is the 
effects of the state change, so if anything we should point that out 
here. But I think just keeping the original message is fine.

> 	}
>
> 	return rv;
>-- 
>2.39.5 (Apple Git-154)

Also, the Fixes tag points to the wrong commit, that was just a 
mechanical change. The actual breakage was introduced in commit
82f59cc63538 ("drbd: fix potential deadlock on detach").