[PATCH] mm/gup: tolerate NULL unlocked in fixup_user_fault()

[Stepan Ionichev]

fixup_user_fault() takes a "bool *unlocked" output parameter that
callers may set to NULL when they do not want the retry/unlock
machinery. The function honours that contract on the way in:

	if (unlocked)
		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;

so callers passing NULL never set FAULT_FLAG_ALLOW_RETRY. In return,
handle_mm_fault() is not expected to produce VM_FAULT_RETRY or
VM_FAULT_COMPLETED for them, which is why the dereferences of
unlocked further down used to be considered unreachable.

That invariant is implicit, not enforced. At least one caller in
arch/s390/pci/pci_mmio.c does pass NULL:

	fixup_user_fault(current->mm, mmio_addr, FAULT_FLAG_WRITE, NULL);

If a future change in handle_mm_fault() ever returned
VM_FAULT_COMPLETED or VM_FAULT_RETRY without ALLOW_RETRY having been
requested, the unconditional "*unlocked = true" stores would
NULL-deref and crash the kernel for this path.

smatch flags both stores:

  mm/gup.c:1597 fixup_user_fault() error: we previously assumed
    'unlocked' could be null (see line 1573)
  mm/gup.c:1612 fixup_user_fault() error: we previously assumed
    'unlocked' could be null (see line 1573)

Make the NULL handling consistent on both sides of the function:
guard the two stores with "if (unlocked)" so fixup_user_fault()
tolerates a NULL output pointer regardless of which fault outcome
handle_mm_fault() returns.

No functional change for callers that already pass a non-NULL
pointer.

Signed-off-by: Stepan Ionichev <sozdayvek@xxxxxxxxx>
---
 mm/gup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index ad9ded396..1a8d7c7c8 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1594,7 +1594,8 @@ int fixup_user_fault(struct mm_struct *mm,
 		 * could tell the callers so they do not need to unlock.
 		 */
 		mmap_read_lock(mm);
-		*unlocked = true;
+		if (unlocked)
+			*unlocked = true;
 		return 0;
 	}
 
@@ -1608,7 +1609,8 @@ int fixup_user_fault(struct mm_struct *mm,
 
 	if (ret & VM_FAULT_RETRY) {
 		mmap_read_lock(mm);
-		*unlocked = true;
+		if (unlocked)
+			*unlocked = true;
 		fault_flags |= FAULT_FLAG_TRIED;
 		goto retry;
 	}
-- 
2.43.0