[PATCH v2] mm/gup: document unlocked invariant in fixup_user_fault

[Nguyen Duy Nhat Anh]

Static analysis tools flag potential NULL pointer
dereferences of 'unlocked' in fixup_user_fault() when handling
VM_FAULT_COMPLETED or VM_FAULT_RETRY.

These warnings are false positives. 'unlocked' is only dereferenced when
handle_mm_fault() returns VM_FAULT_COMPLETED or VM_FAULT_RETRY. Both of
these return codes require FAULT_FLAG_ALLOW_RETRY to be set in
fault_flags, which fixup_user_fault() only sets if 'unlocked' is non-NULL
upon entry. Therefore, if 'unlocked' is NULL, the control flow branches
that dereference 'unlocked' are unreachable.

However, this part of the code is subtle and can trip up
contributors or automated tools. Document this invariant with a comment
above 'if (unlocked)' where fault_flags is constructed, explaining why
omitting FAULT_FLAG_ALLOW_RETRY guarantees 'unlocked' will not be
dereferenced later in the fault recovery loop.

Suggested-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@xxxxxxxxx>
---
v1: https://lore.kernel.org/linux-mm/20261003194846.205918-1-neganhat@xxxxxxxxx/

v2 changes:
- Instead of adding runtime NULL checks at dereference sites,
  document the FAULT_FLAG_ALLOW_RETRY invariant above if (unlocked).
---
 mm/gup.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index eb898ea1ee22..58fa75441da1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1570,6 +1570,12 @@ int fixup_user_fault(struct mm_struct *mm,
 
 	address = untagged_addr_remote(mm, address);
 
+	/*
+	 * If the caller passes 'unlocked' as NULL, FAULT_FLAG_ALLOW_RETRY is omitted.
+	 * This guarantees handle_mm_fault() will never drop the lock or
+	 * return VM_FAULT_COMPLETED / VM_FAULT_RETRY, making subsequent
+	 * dereferences of 'unlocked' unreachable when NULL.
+	 */
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
-- 
2.55.0