Thread (41 messages) 41 messages, 3 authors, 7h ago

Re: [PATCH v3 08/21] md/md-llbitmap: track target reshape geometry fields

From: sashiko-bot@kernel.org
Date: 2026-07-28 09:12:36

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Mismatch between bitmap memory allocation (scaled by the new doubled chunksize) and IO indexing (which continues to use the old smaller chunkshift), leading to an out-of-bounds read/write on the `llbitmap->pctl` array.
- [High] The newly added reshape geometry tracking fields (reshape_sync_size, reshape_chunksize, reshape_chunks) are written to but never applied, causing the bitmap to permanently use the old geometry after an array reshape completes.
--
--- Patch [8]: [PATCH v3 08/21] md/md-llbitmap: track target reshape geometry fields ---
commit 63e12bf7c87e70924898a94a3939282826b19c8e
Author: Yu Kuai [off-list ref]

md/md-llbitmap: track target reshape geometry fields

This commit adds tracking for target reshape geometry in the llbitmap module.
It adds the reshape geometry fields to the llbitmap struct and introduces a
refresh helper. It also updates the load and resize paths to keep the target
geometry in sync.
quoted hunk ↗ jump to hunk
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 1ab9b1689fd80..cdced2126e975 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -430,6 +433,39 @@ static void llbitmap_calculate_chunks(struct mddev *mddev, sector_t blocks,
 	}
 }
 
+static bool llbitmap_reshaping(struct llbitmap *llbitmap)
+{
+	return llbitmap->mddev->reshape_position != MaxSector;
+}
+
+static sector_t llbitmap_personality_sync_size(struct llbitmap *llbitmap,
+					       bool previous)
+{
+	struct mddev *mddev = llbitmap->mddev;
+
+	if (!llbitmap_reshaping(llbitmap) || !mddev->private || !mddev->pers ||
+	    !mddev->pers->bitmap_sync_size)
+		return llbitmap->sync_size;
+	return mddev->pers->bitmap_sync_size(mddev, previous);
+}
+
+static void llbitmap_refresh_reshape(struct llbitmap *llbitmap)
+{
+	unsigned long old_chunks = DIV_ROUND_UP_SECTOR_T(llbitmap->sync_size,
+						 llbitmap->chunksize);
+	sector_t blocks = llbitmap_personality_sync_size(llbitmap, false);
+	unsigned long chunksize = llbitmap->chunksize;
+	unsigned long chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
+
+	llbitmap->reshape_sync_size = blocks;
+	llbitmap->reshape_chunksize = chunksize;
+	llbitmap->reshape_chunks = chunks;
+	llbitmap_calculate_chunks(llbitmap->mddev, blocks,
+				  &llbitmap->reshape_chunksize,
+				  &llbitmap->reshape_chunks);
+	llbitmap->chunks = max(old_chunks, llbitmap->reshape_chunks);
[Severity: Critical]
Can this mismatch between allocation and IO indexing cause an out-of-bounds
access on the pctl array?

When an array is grown significantly, chunksize doubles, and the memory
allocation here scales based on the new larger chunksize. However, IO
indexing in llbitmap_start_write() computes the start using the older,
smaller chunkshift:

drivers/md/md-llbitmap.c:llbitmap_start_write() {
    ...
    unsigned long start = offset >> llbitmap->chunkshift;
    unsigned long end = (offset + sectors - 1) >> llbitmap->chunkshift;
    int page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;
    int page_end = (end + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;

    while (page_start <= page_end) {
        llbitmap_raise_barrier(llbitmap, page_start);
        page_start++;
    }
}

If writes hit the newly expanded region, won't page_start vastly exceed
llbitmap->nr_pages and crash before any bounds-checking can occur?
+}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1284,6 +1322,9 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
 		goto out;
 
 	if (reshape) {
+		llbitmap->reshape_sync_size = blocks;
+		llbitmap->reshape_chunksize = bitmap_chunksize;
+		llbitmap->reshape_chunks = chunks;
[Severity: High]
Does this code ever apply these new geometry tracking fields to the active
state after a reshape completes?

The resize path populates reshape_sync_size, reshape_chunksize, and
reshape_chunks, but there doesn't appear to be any transition logic or
a reshape finish hook to apply them to the active geometry. 

Will array writes to newly grown regions fall outside the bitmap's tracked
chunks and be silently ignored, resulting in a loss of redundancy?
 		llbitmap->chunks = max(old_chunks, chunks);
 	} else {
 		if (blocks < old_blocks && chunks < old_chunks)
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=8
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help