[PATCH] mm/gup: fix NULL pointer dereference in fixup_user_fault()

[PATCH] mm/gup: fix NULL pointer dereference in fixup_user_fault()

[Nguyen Duy Nhat Anh]

In fixup_user_fault(), the 'unlocked' parameter is checked for NULL
early on, allowing callers to pass NULL if they do not track whether the
mmap lock was dropped.

However, if handle_mm_fault() returns VM_FAULT_COMPLETED, line 1597
dereferences 'unlocked' directly (*unlocked = true) without checking
if it is NULL. Callers like s390's pci_mmio.c pass NULL for 'unlocked',
leading to a kernel NULL pointer dereference when VM_FAULT_COMPLETED
occurs.

Fix this by checking if 'unlocked' is non-NULL before assigning to it.
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 mm/gup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index eb898ea1ee22..64f709a45adc 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.55.0

Re: [PATCH] mm/gup: fix NULL pointer dereference in fixup_user_fault()

[Andrew Morton]

On Sun,  4 Oct 2026 02:48:46 +0700 Nguyen Duy Nhat Anh <neganhat@gmail.com> wrote:

> In fixup_user_fault(), the 'unlocked' parameter is checked for NULL
> early on, allowing callers to pass NULL if they do not track whether the
> mmap lock was dropped.
> 
> However, if handle_mm_fault() returns VM_FAULT_COMPLETED, line 1597
> dereferences 'unlocked' directly (*unlocked = true) without checking
> if it is NULL. Callers like s390's pci_mmio.c pass NULL for 'unlocked',
> leading to a kernel NULL pointer dereference when VM_FAULT_COMPLETED
> occurs.
> 
> Fix this by checking if 'unlocked' is non-NULL before assigning to it.

This code is too subtle so you aren't the first to attempt to "fix" it.

The key hint is in the kerneldoc:

 * @unlocked:	did we unlock the mmap_lock while retrying, maybe NULL if caller
 *		does not allow retry. If NULL, the caller must guarantee
 *		that fault_flags does not contain FAULT_FLAG_ALLOW_RETRY.

Trace through the
FAULT_FLAG_ALLOW_RETRY/VM_FAULT_COMPLETED/VM_FAULT_RETRY logic
to confirm that the null deref is a cant-happen.

It would be great if you could pick through all this and propose
addition of a comment which will make this code less confusing for
others.

Thanks.

Re: [PATCH] mm/gup: fix NULL pointer dereference in fixup_user_fault()

[Lance Yang]

On Sun, Oct 04, 2026 at 12:56:01PM -0700, Andrew Morton wrote:
>On Sun,  4 Oct 2026 02:48:46 +0700 Nguyen Duy Nhat Anh <neganhat@gmail.com> wrote:
>
>> In fixup_user_fault(), the 'unlocked' parameter is checked for NULL
>> early on, allowing callers to pass NULL if they do not track whether the
>> mmap lock was dropped.
>> 
>> However, if handle_mm_fault() returns VM_FAULT_COMPLETED, line 1597
>> dereferences 'unlocked' directly (*unlocked = true) without checking
>> if it is NULL. Callers like s390's pci_mmio.c pass NULL for 'unlocked',
>> leading to a kernel NULL pointer dereference when VM_FAULT_COMPLETED
>> occurs.
>> 
>> Fix this by checking if 'unlocked' is non-NULL before assigning to it.
>
>This code is too subtle so you aren't the first to attempt to "fix" it.
>
>The key hint is in the kerneldoc:
>
> * @unlocked:	did we unlock the mmap_lock while retrying, maybe NULL if caller
> *		does not allow retry. If NULL, the caller must guarantee
> *		that fault_flags does not contain FAULT_FLAG_ALLOW_RETRY.
>
>Trace through the
>FAULT_FLAG_ALLOW_RETRY/VM_FAULT_COMPLETED/VM_FAULT_RETRY logic
>to confirm that the null deref is a cant-happen.

IIUC, that's indeed a can't-happen :)