Misuse of mapping_set_folio_order_range()
Matthew Wilcox <[email protected]> Mon, 27 Jul 2026 21:25:01 +0100
| Newsgroups | org.kernel.vger.linux-ext4 |
|---|---|
| Message-ID | <[email protected]> |
While reviewing an unrelated change, Sashiko seems to have found a bug in ext4: > --- Patch [9]: [PATCH v6 09/12] filemap: Remove checks in mapping_set_folio_order_range() --- > commit c0859bbb9afcf450bfdee34a144cba981c50e605 > Author: Matthew Wilcox (Oracle) <[email protected]> > > filemap: Remove checks in mapping_set_folio_order_range() > > This commit removes the maximum order checks in mapping_set_folio_order_range() > to allow its use in hugetlb. > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 2c3718d592d6d..968b791cfd148 100644 > > --- a/include/linux/pagemap.h > > +++ b/include/linux/pagemap.h > > @@ -405,29 +405,23 @@ static inline size_t mapping_max_folio_size_supported(void) > > [ ... ] > > > static inline void mapping_set_folio_order_range(struct address_space *mapping, > > - unsigned int min, > > - unsigned int max) > > + unsigned int min, unsigned int max) > > { > > - if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) > > + if (!IS_ENABLED(CONFIG_LARGE_FOLIO)) > > return; > > - > > - if (min > MAX_PAGECACHE_ORDER) > > - min = MAX_PAGECACHE_ORDER; > > - > > - if (max > MAX_PAGECACHE_ORDER) > > - max = MAX_PAGECACHE_ORDER; > > - > > if (max < min) > > max = min; > > > > mapping->flags = (mapping->flags & ~AS_FOLIO_ORDER_MASK) | > > (min << AS_FOLIO_ORDER_MIN) | (max << AS_FOLIO_ORDER_MAX); > > This is a pre-existing issue, but can this non-atomic read-modify-write of > mapping->flags cause lost updates to concurrently set atomic bit flags? > > While the documentation above this function states it should not be called > while the inode is active, ext4_change_inode_journal_flag() dynamically > invokes it on an active inode during the EXT4_IOC_SETFLAGS ioctl: > > fs/ext4/inode.c:ext4_change_inode_journal_flag() { > ... > ext4_set_inode_mapping_order(inode); > ... > } > > If a privileged user toggles the ext4 journal flag, could this non-atomic > update race with mm_take_all_locks() (which can be triggered by userfaultfd > or KVM) modifying AS_MM_ALL_LOCKS via atomic bitops? > > Since they do not share any locks (ext4_change_inode_journal_flag() holds > inode_lock and invalidate_lock, while mm_take_all_locks() holds mmap_lock > and i_mmap_rwsem), losing the AS_MM_ALL_LOCKS bit could cause > vm_unlock_mapping() to skip unlocking mapping->i_mmap_rwsem, resulting in a > permanent deadlock. Making mapping_set_folio_order_range() atomic would be a pain -- it'd involve a cmpxchg() loop which would be utterly useless for most callers. Can you figure out how to fix this in ext4?