From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6681113A3ED for ; Thu, 30 Apr 2026 05:36:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.217.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777527414; cv=none; b=Q2oG8v89FeMyqjIkLVzZ8RPGfkqRiaRiqhF5zZQEMX5l6OxxmPyPFHJntuT1cHBTnfgIH+LZrb5nag/g/0JUahPa4ULyaHCmbk/mXuhHZQVHyj51fliifaKLpY4X7+L9//9SrXSSNhVjiNb+KeVKyzRgqKo6TOVGNA4ASKRuVRQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777527414; c=relaxed/simple; bh=mdtDdNY2h3DBj2KnBEOFyjFZNYUhiPcjox3rvAtufTY=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=cDyZ6+FR4WMAZY8w/RRn2mePj7N61+ZlRlUMtSJ2zPaMhltWnlzFxD1dOFiPty/fLzh/BAR59SAc7WbH3vgH5pCh4Stnhyz0imO2sjYjJNdujoP92Idjx4B5yWOhLe5a+g2nzWirbCAd7aqAcjv4KCKRtBB0vWskTB8yJPzWws8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BkL2o/qd; arc=none smtp.client-ip=209.85.217.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BkL2o/qd" Received: by mail-vs1-f46.google.com with SMTP id ada2fe7eead31-6121f20650dso192800137.1 for ; Wed, 29 Apr 2026 22:36:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1777527412; x=1778132212; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=ocQlHHH1wPEO06aL5OauXm+y1fwgshk6cQ3uJMAxoO0=; b=BkL2o/qdTUsp/uqx+zLk7ID3+ABFCEcuE0XNvGSb551w5qwENbJm1dCNwhFDWJ7xIE PobWRsQLloR8RNZzcvqzHDCVNonEdIaJTBFKKxZFIjcT1+Ourm/ef+985A2CV3zYMP2E s8BYH40xrGB0HoAETulpNdsIgjNnLm4aCvWZ4mF4uVprGdzBexKUWpgzdWC9sAU95LbC 7fYwA3lRJitFQkKQcQdSyQVNiyG8EkcBwJ4ppR4810382XzRi3hbpVjF1E50Ujhxtd2n 1sYlfhFnR/nyjK9BaRIwj7V5joJk9xoH0VxEsLNWoYLYpe58w4HiTFnlr1t4uqFxpK0d OHpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777527412; x=1778132212; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ocQlHHH1wPEO06aL5OauXm+y1fwgshk6cQ3uJMAxoO0=; b=U0JmUOWMrkPbcqdsy6awsVPXYUw4H448dueSLjN/Pad/Eiqq6C/eCzqaRd8eMAGhmN 0DXgfKdlibWhqU2mf1sFENTsAgdI/Bhee5xvAHESagaaSi7/DZ2TD6MoERUVR3hBn0WV oMNIvMYZTuUZ+RIBaF3D/mMlMqUX+A7zwRh4z/eRtC3UfB8t1MRWlVgXCfwbNyiJHqB+ AuCM2RuaPij24X6+fjZ9q47xLwwCzqTFNToe+WKXWLgrdGiZQtp5YSKSPioNSSD64lcf KmU4uEo0bUifvbMiSF3rdjtS2qC/B4XuPztEXredB9ZLTotNaRtbLtxnNwe5qtnRrXfZ 5jAA== X-Forwarded-Encrypted: i=1; AFNElJ9nP+k9DorKyQVjHAxbyOeTCtFxKPU1doOjZgfIHcrvjWHHhkAQZX7aM1/X0rgnUm+tFVUUjxzUOhPSe5w=@vger.kernel.org X-Gm-Message-State: AOJu0YxqRp0bqAzacjgPBDUXR+iPEKYdnR0WbPzKnkjnhLLuCzir0iC9 pMx701SaBuQwel0tAlXNgZ3xwIt59GUg9HrqmzJiswqa3pxN5YJyviDl X-Gm-Gg: AeBDiesW7rl/7u6cadGYIW1iFCWfvGVNZCzCKJDjVxtXN1ou0mftBPjzzcWnPmR5lGs QYxepmz69d8LrUsDr97VTkYibPtQt+LFZo8BwNrEt/6+Ebq41qRuMPKQ4OARO5fRKSs3CjnZvR5 /RZo3KQsZKMYdUChMdmOcVCZ4lbUP2eOpyPpvkH7yZRbTSZ2TL2huwzCC0Nanq8hMRLYTZJa1SZ vM2U4px5lomzifqGn2+wNmxBOKzZJa4XrCLGoUCSIdW8c/ikj66jc6wlbCaNZrdjsWZE1tYq0K7 bzLz/pLchENSHghuKFYJU7g+5Z9ymLiy9qj0H7MK7ZcEMHhWcsG54nTAW8HNiftaDZewuDufPQh Z6q5UoVmrZVzrX7TEKVs/QmCwSOsLUhukCIPJaVt0GxyoBk7fJ2Ol52HAP226LzL3BEZkrrEaHo e9Qq3N3rCXsBwmLd8eTpyzboZXcJxJFb7RzfHKcoeq/szgQn7jlfz6 X-Received: by 2002:a05:6102:149d:b0:609:5643:e9f2 with SMTP id ada2fe7eead31-62ad1529ba1mr582184137.6.1777527411933; Wed, 29 Apr 2026 22:36:51 -0700 (PDT) Received: from syssplab.cs.fiu.edu (nat1.cs.fiu.edu. [131.94.134.89]) by smtp.gmail.com with ESMTPSA id ada2fe7eead31-62982736ba5sm2411089137.5.2026.04.29.22.36.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Apr 2026 22:36:50 -0700 (PDT) From: Chao Shi To: linux-fsdevel@vger.kernel.org Cc: viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz, willy@infradead.org, linux-kernel@vger.kernel.org, Chao Shi , Sungwoo Kim , Dave Tian , Weidong Zhu Subject: [PATCH v2] fs/buffer: serialize set_buffer_uptodate against concurrent clears Date: Thu, 30 Apr 2026 01:36:45 -0400 Message-ID: <20260430053645.1466196-1-coshi036@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit A WARN_ON_ONCE(!buffer_uptodate(bh)) in mark_buffer_dirty() is reachable from the buffered write path on a block device when the underlying device returns I/O errors at high density. Reproduced by fuzzing an NVMe controller (FEMU) that returns crafted error completions for a sustained workload from /dev/nvme0n1. The contract documented at set_buffer_uptodate() in include/linux/buffer_head.h reads: Any other serialization (with IO errors or whatever that might clear the bit) has to come from other state (eg BH_Lock). In fs/buffer.c, BH_Uptodate can be cleared from four I/O completion callbacks: __end_buffer_read_notouch, end_buffer_write_sync, end_buffer_async_read, end_buffer_async_write. end_buffer_async_read() runs with BH_Lock held throughout, so its clear is already serialized against any caller that also holds BH_Lock around set_buffer_uptodate(); the call may in fact be redundant, but addressing that is independent of this fix. end_buffer_write_sync() likewise holds BH_Lock while it clears BH_Uptodate on the write-error path. Removing that clear would change long-standing buffer-cache I/O-error semantics and is out of scope here. The race is therefore between block_commit_write() and end_buffer_write_sync(): CPU A: block_commit_write CPU B: end_buffer_write_sync (folio lock held, BH_Lock NOT) (BH_Lock held) set_buffer_uptodate(bh); clear_buffer_uptodate(bh); unlock_buffer(bh); mark_buffer_dirty(bh); /* WARN */ CPU B observes the contract; CPU A does not. With one side unlocked the serialization is one-sided and ineffective: CPU A's set can be immediately followed by CPU B's clear, tripping the WARN_ON_ONCE. In the fuzzing reproducer, write-error completions are frequent (visible as repeated "lost async page write" and per-LBA write failures); buffer I/O completion callbacks on the write-error path (e.g. end_buffer_write_sync, end_buffer_async_write) clear BH_Uptodate while holding BH_Lock. The bug is not benign: a not-uptodate buffer can be marked dirty and subsequently written back; depending on whether the buffer was fully or partially covered by the user write, this can leave on-disk content that does not match the intended buffered write state. Fix this by taking BH_Lock around set_buffer_uptodate() + mark_buffer_dirty() in block_commit_write(), so both sides of the contract use the documented serialization. Found by FuzzNvme (Syzkaller with FEMU fuzzing framework). Acked-by: Sungwoo Kim Acked-by: Dave Tian Acked-by: Weidong Zhu Signed-off-by: Chao Shi --- Hi Matthew, Hi Jan, Thanks for the review on v1. v2 takes the feedback, quick notes below. To Matthew: You were right that the v1's timing diagram named the wrong racer. The actual race is with end_buffer_write_sync() on the write-error path, as Jan pointed out -- it also clears BH_Uptodate under BH_Lock, but block_commit_write()'s else branch was reaching set_buffer_uptodate without BH_Lock, leaving the serialization one-sided. v2's commit message and in-code comment now name end_buffer_write_sync() as the racer. To Jan: Thanks for confirming the racer and for the historical context on the dirty + !uptodate state question. v2 keeps the fix scoped to taking BH_Lock in block_commit_write(); the broader semantic question and any change to end_buffer_write_sync()'s clear is left out of scope here. The redundant lock_buffer/unlock_buffer that v1 added to the buffer_new branch of __block_write_begin_int() is also dropped in v2 -- that bh has no in-flight async I/O so no race exists there. v1 thread for context: https://lore.kernel.org/all/20260426020137.1221985-1-coshi036@gmail.com/ Thanks, Chao Changes in v2: - Drop the lock_buffer/unlock_buffer added in v1 to the buffer_new branch of __block_write_begin_int(): that bh is freshly BH_New and has no in-flight async I/O on it, so no race exists at that site. - Rewrite the commit message and the in-code comment to identify end_buffer_write_sync() as the actual racer, not end_buffer_async_read() as v1 claimed; end_buffer_async_read() holds BH_Lock across its clear so a caller that also holds BH_Lock would already be serialized. - Reference the BH_Lock contract at set_buffer_uptodate() in include/linux/buffer_head.h explicitly. - Drop verbose line-number citations and the WARN stack dump from the commit message; tighten wording around reproducer evidence and on-disk impact. v1: https://lore.kernel.org/all/20260426020137.1221985-1-coshi036@gmail.com/ fs/buffer.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fs/buffer.c b/fs/buffer.c index 4d7f84e77d2..6fddb2f1e7c 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2104,8 +2104,22 @@ void block_commit_write(struct folio *folio, size_t from, size_t to) if (!buffer_uptodate(bh)) partial = true; } else { + /* + * Per the contract documented at set_buffer_uptodate() + * in include/linux/buffer_head.h, callers must hold + * BH_Lock to serialize against concurrent clears of + * BH_Uptodate. Holding only the folio lock is not + * sufficient: a concurrent end_buffer_write_sync() on + * the write-error path clears BH_Uptodate while + * holding BH_Lock; without BH_Lock here the clear can + * land between set_buffer_uptodate() and + * mark_buffer_dirty(), tripping the WARN_ON_ONCE in + * mark_buffer_dirty(). + */ + lock_buffer(bh); set_buffer_uptodate(bh); mark_buffer_dirty(bh); + unlock_buffer(bh); } if (buffer_new(bh)) clear_buffer_new(bh); base-commit: ffe69af1f87fa77da975ad4b0b093d48c3cbe6c3 -- 2.43.0