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

[David Hildenbrand (Arm)]

On 10/5/26 04:19, Lance Yang wrote:
> 
> 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@xxxxxxxxx> 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 :)

https://lore.kernel.org/linux-mm/c44e5924-091f-43ed-8cff-34e87c3d9763@xxxxxxxxxx/

People should stop trusting tool output and use their brain ;)

If you did mmap_read_lock() and wouldn't have a way to indicate that to the user
something would be seriously messed up.

-- 
Cheers,

David