* [PATCH] brbd: Fix atomicity violation in drbd_uuid_set_bm()
@ 2024-09-11 9:16 Qiu-ji Chen
2024-09-12 13:19 ` Christoph Hellwig
0 siblings, 1 reply; 2+ messages in thread
From: Qiu-ji Chen @ 2024-09-11 9:16 UTC (permalink / raw)
To: philipp.reisner, lars.ellenberg, christoph.boehmwalder, axboe
Cc: drbd-dev, linux-block, linux-kernel, baijiaju1990, Qiu-ji Chen,
stable
The violation of atomicity occurs when the brbd_uuid_set_bm function is
executed simultaneously with modifying the value of
device->ldev->md.uuid[UI_BITMAP]. Consider a scenario where, while
device->ldev->md.uuid[UI_BITMAP] passes the validity check when its value
is not zero, the value of device->ldev->md.uuid[UI_BITMAP] is written to
zero. In this case, the check in brbd_uuid_set_bm might refer to the old
value of device->ldev->md.uuid[UI_BITMAP] (before locking), which allows
an invalid value to pass the validity check, resulting in inconsistency.
To address this issue, it is recommended to include the data validity check
within the locked section of the function. This modification ensures that
the value of device->ldev->md.uuid[UI_BITMAP] does not change during the
validation process, thereby maintaining its integrity.
This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs to extract
function pairs that can be concurrently executed, and then analyzes the
instructions in the paired functions to identify possible concurrency bugs
including data races and atomicity violations.
Fixes: 9f2247bb9b75 ("drbd: Protect accesses to the uuid set with a spinlock")
Cc: stable@vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666@gmail.com>
---
drivers/block/drbd/drbd_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index a9e49b212341..abafc4edf9ed 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -3399,10 +3399,12 @@ void drbd_uuid_new_current(struct drbd_device *device) __must_hold(local)
void drbd_uuid_set_bm(struct drbd_device *device, u64 val) __must_hold(local)
{
unsigned long flags;
- if (device->ldev->md.uuid[UI_BITMAP] == 0 && val == 0)
+ spin_lock_irqsave(&device->ldev->md.uuid_lock, flags);
+ if (device->ldev->md.uuid[UI_BITMAP] == 0 && val == 0) {
+ spin_unlock_irqrestore(&device->ldev->md.uuid_lock, flags);
return;
+ }
- spin_lock_irqsave(&device->ldev->md.uuid_lock, flags);
if (val == 0) {
drbd_uuid_move_history(device);
device->ldev->md.uuid[UI_HISTORY_START] = device->ldev->md.uuid[UI_BITMAP];
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] brbd: Fix atomicity violation in drbd_uuid_set_bm()
2024-09-11 9:16 [PATCH] brbd: Fix atomicity violation in drbd_uuid_set_bm() Qiu-ji Chen
@ 2024-09-12 13:19 ` Christoph Hellwig
0 siblings, 0 replies; 2+ messages in thread
From: Christoph Hellwig @ 2024-09-12 13:19 UTC (permalink / raw)
To: Qiu-ji Chen
Cc: philipp.reisner, lars.ellenberg, christoph.boehmwalder, axboe,
drbd-dev, linux-block, linux-kernel, baijiaju1990, stable
I can't really comment on the code here, but the subject needs a
s/brbd/drbd/
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-09-12 13:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-11 9:16 [PATCH] brbd: Fix atomicity violation in drbd_uuid_set_bm() Qiu-ji Chen
2024-09-12 13:19 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).