Misuse of mapping_set_folio_order_range()

Misuse of mapping_set_folio_order_range()

[Matthew Wilcox]

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) <willy@infradead.org>
> 
> 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?