Linux kernel -stable discussions
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Sarah Newman" <srn@prgmr.com>,
	"Lars Ellenberg" <lars@linbit.com>,
	"Christoph Böhmwalder" <christoph.boehmwalder@linbit.com>,
	"Jens Axboe" <axboe@kernel.dk>, "Sasha Levin" <sashal@kernel.org>,
	philipp.reisner@linbit.com, lars.ellenberg@linbit.com,
	drbd-dev@lists.linbit.com
Subject: [PATCH AUTOSEL 6.12 18/31] drbd: add missing kref_get in handle_write_conflicts
Date: Sun,  3 Aug 2025 17:19:21 -0400	[thread overview]
Message-ID: <20250803211935.3547048-18-sashal@kernel.org> (raw)
In-Reply-To: <20250803211935.3547048-1-sashal@kernel.org>

From: Sarah Newman <srn@prgmr.com>

[ Upstream commit 00c9c9628b49e368d140cfa61d7df9b8922ec2a8 ]

With `two-primaries` enabled, DRBD tries to detect "concurrent" writes
and handle write conflicts, so that even if you write to the same sector
simultaneously on both nodes, they end up with the identical data once
the writes are completed.

In handling "superseeded" writes, we forgot a kref_get,
resulting in a premature drbd_destroy_device and use after free,
and further to kernel crashes with symptoms.

Relevance: No one should use DRBD as a random data generator, and apparently
all users of "two-primaries" handle concurrent writes correctly on layer up.
That is cluster file systems use some distributed lock manager,
and live migration in virtualization environments stops writes on one node
before starting writes on the other node.

Which means that other than for "test cases",
this code path is never taken in real life.

FYI, in DRBD 9, things are handled differently nowadays.  We still detect
"write conflicts", but no longer try to be smart about them.
We decided to disconnect hard instead: upper layers must not submit concurrent
writes. If they do, that's their fault.

Signed-off-by: Sarah Newman <srn@prgmr.com>
Signed-off-by: Lars Ellenberg <lars@linbit.com>
Signed-off-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
Link: https://lore.kernel.org/r/20250627095728.800688-1-christoph.boehmwalder@linbit.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my analysis of the commit and the surrounding code context,
here is my assessment:

**YES** - This commit should be backported to stable kernel trees.

Here's my extensive explanation:

1. **Critical Bug Fix**: This commit fixes a missing `kref_get` that
   causes a use-after-free vulnerability. The code at
   drivers/block/drbd/drbd_receiver.c:2503-2507 shows the fix adds the
   missing reference count increment before queuing work that will later
   decrement the reference count in `drbd_send_acks_wf()`.

2. **Clear Use-After-Free Pattern**: Looking at the code pattern:
   - Before the fix: The code queues work (`queue_work`) that will call
     `drbd_send_acks_wf()`, which contains `kref_put(&device->kref,
     drbd_destroy_device)` at line 6119
   - Without the matching `kref_get`, this leads to premature
     destruction of the device structure
   - The commit message explicitly states this results in "premature
     drbd_destroy_device and use after free"

3. **Follows Established Pattern**: The fix follows the exact same
   pattern used elsewhere in the DRBD code. At
   drivers/block/drbd/drbd_worker.c:150-152, we see:
  ```c
  kref_get(&device->kref); /* put is in drbd_send_acks_wf() */
  if (!queue_work(connection->ack_sender, &peer_device->send_acks_work))
  kref_put(&device->kref, drbd_destroy_device);
  ```
  This shows the fix is correct and consistent with the codebase.

4. **Small and Contained Fix**: The change is minimal (adding 4 lines),
   well-understood, and doesn't introduce any architectural changes. It
   simply ensures proper reference counting.

5. **Real Security Issue**: Use-after-free bugs can lead to kernel
   crashes and potentially be exploited for privilege escalation. Even
   though the commit notes this code path is rarely taken in production
   ("two-primaries" mode with concurrent writes), when it is triggered,
   it causes kernel crashes.

6. **Meets Stable Criteria**: According to Documentation/process/stable-
   kernel-rules.rst:
   - It fixes "a real bug that bothers people" (kernel crashes)
   - It's "obviously correct and tested" (follows established pattern)
   - It's under 100 lines
   - It fixes "data corruption, a real security issue" (use-after-free)

7. **No Side Effects**: The fix only adds proper reference counting and
   doesn't change any logic or introduce new features. The only behavior
   change is preventing the use-after-free bug.

The fact that this affects a rarely-used code path ("two-primaries"
mode) doesn't diminish its importance for backporting, as users who do
use this feature would experience kernel crashes without this fix. The
fix is safe, minimal, and prevents a serious bug.

 drivers/block/drbd/drbd_receiver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 0c9f54197768..ac18d36b0ea8 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2500,7 +2500,11 @@ static int handle_write_conflicts(struct drbd_device *device,
 			peer_req->w.cb = superseded ? e_send_superseded :
 						   e_send_retry_write;
 			list_add_tail(&peer_req->w.list, &device->done_ee);
-			queue_work(connection->ack_sender, &peer_req->peer_device->send_acks_work);
+			/* put is in drbd_send_acks_wf() */
+			kref_get(&device->kref);
+			if (!queue_work(connection->ack_sender,
+					&peer_req->peer_device->send_acks_work))
+				kref_put(&device->kref, drbd_destroy_device);
 
 			err = -ENOENT;
 			goto out;
-- 
2.39.5


  parent reply	other threads:[~2025-08-03 21:20 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-03 21:19 [PATCH AUTOSEL 6.12 01/31] hfs: fix general protection fault in hfs_find_init() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 02/31] hfs: fix slab-out-of-bounds in hfs_bnode_read() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 03/31] hfsplus: fix slab-out-of-bounds in hfsplus_bnode_read() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 04/31] hfsplus: fix slab-out-of-bounds read in hfsplus_uni2asc() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 05/31] hfsplus: don't use BUG_ON() in hfsplus_create_attributes_file() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 06/31] arm64: Handle KCOV __init vs inline mismatches Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 07/31] firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 08/31] smb/server: avoid deadlock when linking with ReplaceIfExists Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 09/31] nvme-pci: try function level reset on init failure Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 10/31] dm-stripe: limit chunk_sectors to the stripe size Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 11/31] md/raid10: set chunk_sectors limit Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 12/31] nvme-tcp: log TLS handshake failures at error level Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 13/31] gfs2: Validate i_depth for exhash directories Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 14/31] gfs2: Set .migrate_folio in gfs2_{rgrp,meta}_aops Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 15/31] md: call del_gendisk in control path Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 16/31] loop: Avoid updating block size under exclusive owner Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 17/31] udf: Verify partition map count Sasha Levin
2025-08-03 21:19 ` Sasha Levin [this message]
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 19/31] hfs: fix not erasing deleted b-tree node issue Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 20/31] better lockdep annotations for simple_recursive_removal() Sasha Levin
2025-08-03 21:19 ` [PATCH AUTOSEL 6.12 21/31] ata: ahci: Disallow LPM policy control if not supported Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250803211935.3547048-18-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=christoph.boehmwalder@linbit.com \
    --cc=drbd-dev@lists.linbit.com \
    --cc=lars.ellenberg@linbit.com \
    --cc=lars@linbit.com \
    --cc=patches@lists.linux.dev \
    --cc=philipp.reisner@linbit.com \
    --cc=srn@prgmr.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox