From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: Unwiring incorrect pages
To: Martin Pieuchot <mpi@grenadille.net>
Cc: tech@openbsd.org
Date: Thu, 25 Dec 2025 13:02:12 +0100

> Date: Thu, 25 Dec 2025 12:02:13 +0100
> From: Martin Pieuchot <mpi@grenadille.net>
> 
> The multiple Syzkaller reports about wired vs managed pages corruption
> stopped after I added some KASSERT()s to uvm_pageunwire().  Instead the
> syscall fuzzer started to report the following:
>   https://syzkaller.appspot.com/bug?extid=db99726653fba0697bd8
> 
> The trace shows a double/incorrect unwiring in the error code path of
> uvm_map_pageable_wire().
> 
> It is still unclear to me what causes this.  That said, I'd like to
> commit the diff below to reduce the number of possible errors: do not
> wire entries that have been already wired.
> 
> ok?

Yes it is silly to call uvm_fault_wire() for entries that are already
wired.  The wired_count is only ever changed when we hold a write map
on the lock.  And while we release the write map in this function, wo
hold the map busy, so the wired_count cannot change between the
wired_count++ in the earlier loop and us checking it here.

ok kettenis@

> Index: uvm/uvm_map.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> diff -u -p -r1.352 uvm_map.c
> --- uvm/uvm_map.c	22 Dec 2025 10:55:07 -0000	1.352
> +++ uvm/uvm_map.c	25 Dec 2025 10:46:10 -0000
> @@ -2116,14 +2116,18 @@ uvm_map_pageable_wire(struct vm_map *map
>  	vm_map_unlock(map);
>  
>  	error = 0;
> -	for (iter = first; error == 0 && iter != end;
> +	for (iter = first; iter != end;
>  	    iter = RBT_NEXT(uvm_map_addr, iter)) {
>  		if (UVM_ET_ISHOLE(iter) || iter->start == iter->end ||
>  		    iter->protection == PROT_NONE)
>  			continue;
>  
> -		error = uvm_fault_wire(map, iter->start, iter->end,
> -		    iter->protection);
> +		if (iter->wired_count == 1) {
> +			error = uvm_fault_wire(map, iter->start, iter->end,
> +			    iter->protection);
> +			if (error)
> +				break;
> +		}
>  	}
>  
>  	vm_map_lock(map);
> @@ -2137,7 +2141,8 @@ uvm_map_pageable_wire(struct vm_map *map
>  
>  		/*
>  		 * first is no longer needed to restart loops.
> -		 * Use it as iterator to unmap successful mappings.
> +		 * Use it as iterator to unwire entries that were
> +		 * successfully wired above.
>  		 */
>  		for (; first != iter;
>  		    first = RBT_NEXT(uvm_map_addr, first)) {
> 
> 
>