Index | Thread | Search

From:
Jonathan Gray <jsg@jsg.id.au>
Subject:
Re: Race in i386 pmap
To:
tech@openbsd.org
Date:
Fri, 15 Nov 2024 22:49:30 +1100

Download raw body.

Thread
On Fri, Nov 15, 2024 at 12:27:53PM +0100, Martin Pieuchot wrote:
> sthen@ reported the following panic:
> 
>   panic: mutex 0xd0fcb50c not held in mtx_leave
>   Stopped at      db_enter+0x4:   popl    %ebp
>       TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
>    355840  43269     55   0x1100000          0    1  sh
>    479702  53077     55   0x1100002          0    6  sh
>    346914  53425      0        0x11          0    4  perl
>    127401  58899     55   0x1000002          0    7  llvm-tblgen
>    302969  18534      0    0x100003        0x8    5  ssh
>    175851  28739      0     0x14000      0x200    0  reaper
>   *253557  31503      0     0x14000      0x200    3  pagedaemon
>    281667  82587      0     0x14000      0x200    2  softnet0
>   db_enter() at db_enter+0x4
>   panic(d0cbff85) at panic+0x7a
>   mtx_leave(d0fcb50c) at mtx_leave+0x6e
>   pmap_page_remove_pae(d403a328) at pmap_page_remove_pae+0x380
>   uvmpd_scan_active(0,0,6) at uvmpd_scan_active+0x14f
>   uvmpd_scan(0,fffffea2,6) at uvmpd_scan+0x73
>   uvm_pageout(d6c0faf8) at uvm_pageout+0x282
> 
> The mutex not held correspond to the `pm_apte_mtx' line 559 inside
> pmap_unmap_ptes_pae() which is inline in pmap_page_remove_pae().
> 
> There's an obvious race in this function which is not present in the
> other x86 pmaps.
> 
> ok?

ok jsg@

a mistake in pmapae.c rev 1.46
'Fix a race in pmap_page_remove_86() and pmap_page_remove_pae().'

> 
> Index: arch/i386/i386/pmapae.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/pmapae.c,v
> diff -u -p -r1.74 pmapae.c
> --- arch/i386/i386/pmapae.c	8 Nov 2024 13:18:29 -0000	1.74
> +++ arch/i386/i386/pmapae.c	15 Nov 2024 11:18:54 -0000
> @@ -1347,7 +1347,7 @@ pmap_page_remove_pae(struct vm_page *pg)
>  		pm = pve->pv_pmap;
>  		mtx_leave(&pg->mdpage.pv_mtx);
>  
> -		ptes = pmap_map_ptes_pae(pve->pv_pmap);	/* locks pmap */
> +		ptes = pmap_map_ptes_pae(pm);	/* locks pmap */
>  
>  		/*
>  		 * We dropped the pvlist lock before grabbing the pmap
> 
>