Re: [PATCH] arch/s390/pci: fix fixup_user_fault() calls with NULL unlocked parameter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Changing the 'unlocked' parameter of fixup_user_fault() from NULL to &unlocked allows the mmap_lock to be temporarily dropped, causing a Use-After-Free of the VMA pointer.
--

commit 440b0dda47238ff5981413f278a2a1abe962cdd0
Author: Liu Dalin <liudalin@kylinsec.com.cn>
arch/s390/pci: fix fixup_user_fault() calls with NULL unlocked parameter

This patch provides a valid unlocked variable to fixup_user_fault() in
the s390 PCI MMIO read and write handlers to properly track the lock state,
resolving an issue where the lock state was obfuscated by passing NULL.

> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
> index f3f79ba78410a..5ef2d6436d4b1 100644
> --- a/arch/s390/pci/pci_mmio.c
> +++ b/arch/s390/pci/pci_mmio.c
> @@ -182,7 +182,9 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>  	args.vma = vma;
>  	ret = follow_pfnmap_start(&args);
>  	if (ret) {
> -		fixup_user_fault(current->mm, mmio_addr, FAULT_FLAG_WRITE, NULL);
> +		bool unlocked = false;
> +
> +		fixup_user_fault(current->mm, mmio_addr, FAULT_FLAG_WRITE, &unlocked);
>  		ret = follow_pfnmap_start(&args);

[Severity: Critical]
If fixup_user_fault() drops the mmap_read_lock and sets unlocked to true,
a concurrent munmap() could free the VMA. Since args.vma was populated
earlier, calling follow_pfnmap_start(&args) immediately after would 
dereference the freed VMA pointer:

mm/memory.c:follow_pfnmap_start() {
	struct vm_area_struct *vma = args->vma;
	...
	struct mm_struct *mm = vma->vm_mm;

Can this regression be avoided by checking the unlocked state and handling
the possibility that the VMA is no longer valid?

>  		if (ret)
>  			goto out_unlock_mmap;

[ ... ]

> @@ -335,7 +337,9 @@ SYSCALL_DEFINE3(s390_pci_mmio_read, unsigned long, mmio_addr,
>  	args.address = mmio_addr;
>  	ret = follow_pfnmap_start(&args);
>  	if (ret) {
> -		fixup_user_fault(current->mm, mmio_addr, 0, NULL);
> +		bool unlocked = false;
> +
> +		fixup_user_fault(current->mm, mmio_addr, 0, &unlocked);
>  		ret = follow_pfnmap_start(&args);

[Severity: Critical]
Similarly in s390_pci_mmio_read(), if the lock is temporarily dropped by
fixup_user_fault(), can this second call to follow_pfnmap_start() cause a
use-after-free regression by accessing the stale args.vma pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0726CF177011E0E2+20260826064228.3255764-1-liudalin@kylinsec.com.cn?part=1