Skip to content

Commit 7e060df

Browse files
Kemeng Shiakpm00
authored andcommitted
Xarray: do not return sibling entries from xas_find_marked()
Patch series "Fixes and cleanups to xarray", v5. This series contains some random fixes and cleanups to xarray. Patch 1-2 are fixes and patch 3-6 are cleanups. More details can be found in respective patches. This patch (of 5): Similar to issue fixed in commit cbc0285 ("XArray: Do not return sibling entries from xa_load()"), we may return sibling entries from xas_find_marked as following: Thread A: Thread B: xa_store_range(xa, entry, 6, 7, gfp); xa_set_mark(xa, 6, mark) XA_STATE(xas, xa, 6); xas_find_marked(&xas, 7, mark); offset = xas_find_chunk(xas, advance, mark); [offset is 6 which points to a valid entry] xa_store_range(xa, entry, 4, 7, gfp); entry = xa_entry(xa, node, 6); [entry is a sibling of 4] if (!xa_is_node(entry)) return entry; Skip sibling entry like xas_find() does to protect caller from seeing sibling entry from xas_find_marked() or caller may use sibling entry as a valid entry and crash the kernel. Besides, load_race() test is modified to catch mentioned issue and modified load_race() only passes after this fix is merged. Here is an example how this bug could be triggerred in tmpfs which enables large folio in mapping: Let's take a look at involved racer: 1. How pages could be created and dirtied in shmem file. write ksys_write vfs_write new_sync_write shmem_file_write_iter generic_perform_write shmem_write_begin shmem_get_folio shmem_allowable_huge_orders shmem_alloc_and_add_folios shmem_alloc_folio __folio_set_locked shmem_add_to_page_cache XA_STATE_ORDER(..., index, order) xax_store() shmem_write_end folio_mark_dirty() 2. How dirty pages could be deleted in shmem file. ioctl do_vfs_ioctl file_ioctl ioctl_preallocate vfs_fallocate shmem_fallocate shmem_truncate_range shmem_undo_range truncate_inode_folio filemap_remove_folio page_cache_delete xas_store(&xas, NULL); 3. How dirty pages could be lockless searched sync_file_range ksys_sync_file_range __filemap_fdatawrite_range filemap_fdatawrite_wbc do_writepages writeback_use_writepage writeback_iter writeback_get_folio filemap_get_folios_tag find_get_entry folio = xas_find_marked() folio_try_get(folio) Kernel will crash as following: 1.Create 2.Search 3.Delete /* write page 2,3 */ write ... shmem_write_begin XA_STATE_ORDER(xas, i_pages, index = 2, order = 1) xa_store(&xas, folio) shmem_write_end folio_mark_dirty() /* sync page 2 and page 3 */ sync_file_range ... find_get_entry folio = xas_find_marked() /* offset will be 2 */ offset = xas_find_chunk() /* delete page 2 and page 3 */ ioctl ... xas_store(&xas, NULL); /* write page 0-3 */ write ... shmem_write_begin XA_STATE_ORDER(xas, i_pages, index = 0, order = 2) xa_store(&xas, folio) shmem_write_end folio_mark_dirty(folio) /* get sibling entry from offset 2 */ entry = xa_entry(.., 2) /* use sibling entry as folio and crash kernel */ folio_try_get(folio) Link: https://lkml.kernel.org/r/[email protected] Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Kemeng Shi <[email protected]> Cc: Mattew Wilcox <[email protected]> [English fixes] Signed-off-by: Andrew Morton <[email protected]>
1 parent cb7c77e commit 7e060df

File tree

2 files changed

+6
-0
lines changed

2 files changed

+6
-0
lines changed

lib/xarray.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,6 +1387,8 @@ void *xas_find_marked(struct xa_state *xas, unsigned long max, xa_mark_t mark)
13871387
entry = xa_entry(xas->xa, xas->xa_node, xas->xa_offset);
13881388
if (!entry && !(xa_track_free(xas->xa) && mark == XA_FREE_MARK))
13891389
continue;
1390+
if (xa_is_sibling(entry))
1391+
continue;
13901392
if (!xa_is_node(entry))
13911393
return entry;
13921394
xas->xa_node = xa_to_node(entry);

tools/testing/radix-tree/multiorder.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ static void *load_creator(void *ptr)
227227
unsigned long index = (3 << RADIX_TREE_MAP_SHIFT) -
228228
(1 << order);
229229
item_insert_order(tree, index, order);
230+
xa_set_mark(tree, index, XA_MARK_1);
230231
item_delete_rcu(tree, index);
231232
}
232233
}
@@ -242,8 +243,11 @@ static void *load_worker(void *ptr)
242243

243244
rcu_register_thread();
244245
while (!stop_iteration) {
246+
unsigned long find_index = (2 << RADIX_TREE_MAP_SHIFT) + 1;
245247
struct item *item = xa_load(ptr, index);
246248
assert(!xa_is_internal(item));
249+
item = xa_find(ptr, &find_index, index, XA_MARK_1);
250+
assert(!xa_is_internal(item));
247251
}
248252
rcu_unregister_thread();
249253

0 commit comments

Comments
 (0)