Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: Fix: pool_get(9) doesn't respect PR_NOWAIT
To:
tech@openbsd.org, dlg@openbsd.org, kettenis@openbsd.org
Date:
Tue, 17 Feb 2026 18:03:54 -0700

Download raw body.

Thread
This is parked until another the uvm_lock_pageq problem is solved.

Martin Pieuchot <mpi@grenadille.net> wrote:

> As reported last month by a couple [0] of people [1] it is possible for
> a pool_get(9) call with PR_NOWAIT to end up sleeping:
> 
> 	panic() at assertwaitok+0xb8
> 	assertwaitok() at pool_get+0x34
> 	pool_get() at uvm_mapent_alloc+0x20c
> 	uvm_mapent_alloc() at uvm_map_clip_start+0x80
> 	uvm_map_clip_start() at uvm_unmap_remove+0x248
> 	uvm_unmap_remove() at uvm_unmap+0x64
> 	uvm_unmap() at km_free+0x50
> 	km_free() at pool_p_alloc+0x1f4
> 	pool_p_alloc() at pool_do_get+0x20c
> 	pool_do_get() at pool_get+0x8c
> 	pool_get() at pmap_vp_enter+0x17c
> 	pmap_vp_enter() at pmap_enter+0x1ac
> 	pmap_enter() at uvm_fault_lower+0x220
> 	uvm_fault_lower() at uvm_fault+0x158
> 	uvm_fault() at udata_abort+0x128
> 
> Diff below fixes that by allocating the pool page header *before* the
> actual page.
> 
> This was enough to fix the issue, although I left a FIXME because the
> pool_put() below might also end up returning a page to UVM which might
> still want to sleep.  Returning the page should IMHO be delayed, at
> least in the PR_NOWAIT case.
> 
> ok?
> 
> [0] https://marc.info/?l=openbsd-bugs&m=176831735127482&w=2
> [1] https://marc.info/?l=openbsd-bugs&m=176839968920955&w=2
> 
> Index: kern/subr_pool.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_pool.c,v
> diff -u -p -r1.242 subr_pool.c
> --- kern/subr_pool.c	1 Aug 2025 19:00:38 -0000	1.242
> +++ kern/subr_pool.c	25 Jan 2026 13:04:25 -0000
> @@ -923,19 +923,28 @@ pool_p_alloc(struct pool *pp, int flags,
>  	pl_assert_unlocked(pp, &pp->pr_lock);
>  	KASSERT(pp->pr_size >= sizeof(*pi));
>  
> +	if (!POOL_INPGHDR(pp)) {
> +		ph = pool_get(&phpool, flags);
> +		if (ph == NULL)
> +			return (NULL);
> +	}
> +
>  	addr = pool_allocator_alloc(pp, flags, slowdown);
> -	if (addr == NULL)
> +	if (addr == NULL) {
> +		if (!POOL_INPGHDR(pp)) {
> +			/*
> +			 * FIXME: this can result in pool_p_free() calling
> +			 * pool_allocator_free() then calling km_free(9).
> +			 * km_free(9) might need to sleep and this is not
> +			 * allowed with PR_NOWAIT.
> +			 */
> +			pool_put(&phpool, ph);
> +		}
>  		return (NULL);
> +	}
>  
>  	if (POOL_INPGHDR(pp))
>  		ph = (struct pool_page_header *)(addr + pp->pr_phoffset);
> -	else {
> -		ph = pool_get(&phpool, flags);
> -		if (ph == NULL) {
> -			pool_allocator_free(pp, addr);
> -			return (NULL);
> -		}
> -	}
>  
>  	XSIMPLEQ_INIT(&ph->ph_items);
>  	ph->ph_page = addr;
> 
>