Re: [PATCH] fs/proc/task_mmu: move mmu notification mechanism inside mm lock

[Andrew Morton]

On Tue, 9 Jan 2024 08:28:06 -0800 Sean Christopherson <seanjc@xxxxxxxxxx> wrote:

> > -	/* Protection change for the range is going to happen. */
> > -	if (p.arg.flags & PM_SCAN_WP_MATCHING) {
> > -		mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_VMA, 0,
> > -					mm, p.arg.start, p.arg.end);
> > -		mmu_notifier_invalidate_range_start(&range);
> > -	}
> > -
> >  	for (walk_start = p.arg.start; walk_start < p.arg.end;
> >  			walk_start = p.arg.walk_end) {
> >  		long n_out;
> 
> Nit, might be worth moving
> 
> 		struct mmu_notifier_range range;
> 
> inside the loop to guard against stale usage, but that's definitely optional.

Yes, I think that's nicer.

--- a/fs/proc/task_mmu.c~fs-proc-task_mmu-move-mmu-notification-mechanism-inside-mm-lock-fix
+++ a/fs/proc/task_mmu.c
@@ -2432,7 +2432,6 @@ static long pagemap_scan_flush_buffer(st
 
 static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
 {
-	struct mmu_notifier_range range;
 	struct pagemap_scan_private p = {0};
 	unsigned long walk_start;
 	size_t n_ranges_out = 0;
@@ -2450,6 +2449,7 @@ static long do_pagemap_scan(struct mm_st
 
 	for (walk_start = p.arg.start; walk_start < p.arg.end;
 			walk_start = p.arg.walk_end) {
+		struct mmu_notifier_range range;
 		long n_out;
 
 		if (fatal_signal_pending(current)) {
_


I'm surprised this code doesn't generate a might-be-used-uninitialized
warning.  I guess gcc got smarter.