The hfsplus_block_allocate() and hfsplus_block_free() kmap the allocation bitmap's pages and modify their bits in place under sbi->alloc_mutex, but without holding the page lock. That leaves the read-modify-write of the bitmap bits unprotected against a concurrent writeback of the same page, which can read a partially-updated bitmap word or race with the dirty-bit update. v2 Christoph Hellwig has detected that taking the page lock around the kmap/modify/kunmap section is not enough on its own: writeback drops the page lock before the write actually completes, so a mutator that only waits on the lock can still start rewriting a page whose old contents are still in flight to the device. Mark the allocation file's mapping with mapping_set_stable_writes() and call folio_wait_stable() right after taking the page lock in both functions, so a mutator also waits out any writeback that was already in progress when it acquired the lock. Signed-off-by: Viacheslav Dubeyko cc: Christoph Hellwig cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org --- fs/hfsplus/bitmap.c | 18 ++++++++++++++++++ fs/hfsplus/super.c | 1 + 2 files changed, 19 insertions(+) diff --git a/fs/hfsplus/bitmap.c b/fs/hfsplus/bitmap.c index 1b3af8c87cad..61c49cca4a7a 100644 --- a/fs/hfsplus/bitmap.c +++ b/fs/hfsplus/bitmap.c @@ -39,6 +39,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, start = size; goto out; } + lock_page(page); + folio_wait_stable(page_folio(page)); pptr = kmap_local_page(page); curr = pptr + (offset & (PAGE_CACHE_BITS - 1)) / 32; i = offset % 32; @@ -75,6 +77,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, curr++; } kunmap_local(pptr); + unlock_page(page); offset += PAGE_CACHE_BITS; if (offset >= size) break; @@ -84,6 +87,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, start = size; goto out; } + lock_page(page); + folio_wait_stable(page_folio(page)); curr = pptr = kmap_local_page(page); if ((size ^ offset) / PAGE_CACHE_BITS) end = pptr + PAGE_CACHE_BITS / 32; @@ -98,6 +103,9 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, start = offset + (curr - pptr) * 32 + i; if (start >= size) { hfs_dbg("bitmap full\n"); + kunmap_local(pptr); + unlock_page(page); + start = size; goto out; } /* do any partial u32 at the start */ @@ -128,6 +136,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, } set_page_dirty(page); kunmap_local(pptr); + unlock_page(page); offset += PAGE_CACHE_BITS; page = read_mapping_page(mapping, offset / PAGE_CACHE_BITS, NULL); @@ -135,6 +144,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, start = size; goto out; } + lock_page(page); + folio_wait_stable(page_folio(page)); pptr = kmap_local_page(page); curr = pptr; end = pptr + PAGE_CACHE_BITS / 32; @@ -152,6 +163,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size, *curr = cpu_to_be32(n); set_page_dirty(page); kunmap_local(pptr); + unlock_page(page); *max = offset + (curr - pptr) * 32 + i - start; sbi->free_blocks -= *max; hfsplus_mark_mdb_dirty(sb); @@ -185,6 +197,8 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count) page = read_mapping_page(mapping, pnr, NULL); if (IS_ERR(page)) goto kaboom; + lock_page(page); + folio_wait_stable(page_folio(page)); pptr = kmap_local_page(page); curr = pptr + (offset & (PAGE_CACHE_BITS - 1)) / 32; end = pptr + PAGE_CACHE_BITS / 32; @@ -216,9 +230,12 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count) break; set_page_dirty(page); kunmap_local(pptr); + unlock_page(page); page = read_mapping_page(mapping, ++pnr, NULL); if (IS_ERR(page)) goto kaboom; + lock_page(page); + folio_wait_stable(page_folio(page)); pptr = kmap_local_page(page); curr = pptr; end = pptr + PAGE_CACHE_BITS / 32; @@ -232,6 +249,7 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count) out: set_page_dirty(page); kunmap_local(pptr); + unlock_page(page); sbi->free_blocks += len; hfsplus_mark_mdb_dirty(sb); mutex_unlock(&sbi->alloc_mutex); diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 5777e31de45a..459ca6f3b814 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -571,6 +571,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) goto out_close_attr_tree; } sbi->alloc_file = inode; + mapping_set_stable_writes(inode->i_mapping); /* Load the root directory */ root = hfsplus_iget(sb, HFSPLUS_ROOT_CNID); -- 2.43.0