Re: [PATCH] mempolicy: migration attempt to match interleave nodes: fix

[Liam R. Howlett]

* Hugh Dickins <hughd@xxxxxxxxxx> [231024 02:50]:
> mm-unstable commit edd33b8807a1 ("mempolicy: migration attempt to match
> interleave nodes") added a second vma_iter search to do_mbind(), to
> determine the interleave index to be used in the MPOL_INTERLEAVE case.
> 
> But sadly it added it just after the mmap_write_unlock(), leaving this
> new VMA search unprotected: and so syzbot reports suspicious RCU usage
> from lib/maple_tree.c:856.
> 
> This could be fixed with an rcu_read_lock/unlock() pair (per Liam);
> but since we have been relying on the mmap_lock up to this point, it's
> slightly better to extend it over the new search too, for a well-defined
> result consistent with the policy this mbind() is establishing (rather
> than whatever might follow once the mmap_lock is dropped).

Would downgrading the lock work?  It would avoid the potential writing
issue and should still satisfy lockdep.

> 
> Reported-by: syzbot+79fcba037b6df73756d3@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/linux-mm/000000000000c05f1b0608657fde@xxxxxxxxxx/
> Fixes: edd33b8807a1 ("mempolicy: migration attempt to match interleave nodes")
> Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
> ---
>  mm/mempolicy.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 989293180eb6..5e472e6e0507 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1291,8 +1291,6 @@ static long do_mbind(unsigned long start, unsigned long len,
>  		}
>  	}
>  
> -	mmap_write_unlock(mm);
> -
>  	if (!err && !list_empty(&pagelist)) {
>  		/* Convert MPOL_DEFAULT's NULL to task or default policy */
>  		if (!new) {
> @@ -1334,7 +1332,11 @@ static long do_mbind(unsigned long start, unsigned long len,
>  				mmpol.ilx -= page->index >> order;
>  			}
>  		}
> +	}
>  
> +	mmap_write_unlock(mm);
> +
> +	if (!err && !list_empty(&pagelist)) {
>  		nr_failed |= migrate_pages(&pagelist,
>  				alloc_migration_target_by_mpol, NULL,
>  				(unsigned long)&mmpol, MIGRATE_SYNC,
> -- 
> 2.35.3
>