On 01/03/24(Fri) 00:09, Alexander Bluhm wrote:
> Hi,
> 
> With witness and lock debug I have seen a crashes on 7.4.  Without
> debug kernel the machine just hangs.
> 
> panic: acquiring blockable sleep lock with spinlock or critical section held (kernel_lock) &kernel_lock
> Stopped at      db_enter+0x14:  popq    %rbp
>     TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
>   90996   9372   3189    0x100003          0    4  gzip
> *274329  88028   3189         0x3          0    0  perl
>  414521  64780   3189         0x3          0    2  perl
> db_enter() at db_enter+0x14
> panic(ffffffff820c8388) at panic+0xc3
> witness_checkorder(ffffffff825dcb70,9,0) at witness_checkorder+0xb61
> __mp_lock(ffffffff825dc968) at __mp_lock+0x63
> intr_handler(ffff80002400f730,ffff800000320180) at intr_handler+0x48
> Xintr_ioapic_edge21_untramp() at Xintr_ioapic_edge21_untramp+0x18f
> pool_do_get(ffffffff82519e58,1,ffff80002400f8a4) at pool_do_get+0x7e
> pool_get(ffffffff82519e58,1) at pool_get+0xa4
> sys_ypconnect(ffff800024372aa0,ffff80002400fb40,ffff80002400fba0) at sys_ypconn
> ect+0xa9
> syscall(ffff80002400fc10) at syscall+0x3d4
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7345666e6710, count: 4
> 
> Pool namei_pool is initialized with IPL_NONE as filesystem always
> runs with kernel lock.  So pool_get() also needs kernel lock in
> sys_ypconnect().
> 
> ok?

I wonder if we should not add a warning inside pool_get().  If the ipl
of the pool is IPL_NONE *and* the KERNEL_LOCK() is not held, then emit
a warning.  Because I've seen this bug too often...

> Index: kern/uipc_syscalls.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_syscalls.c,v
> diff -u -p -r1.217 uipc_syscalls.c
> --- kern/uipc_syscalls.c	3 Feb 2024 22:50:09 -0000	1.217
> +++ kern/uipc_syscalls.c	27 Feb 2024 17:51:22 -0000
> @@ -1560,12 +1560,12 @@ sys_ypconnect(struct proc *p, void *v, r
>  
>  	if (p->p_p->ps_flags & PS_CHROOT)
>  		return EACCES;
> +	KERNEL_LOCK();
>  	name = pool_get(&namei_pool, PR_WAITOK);
>  	snprintf(name, MAXPATHLEN, "/var/yp/binding/%s.2", domainname);
>  	NDINIT(&nid, 0, NOFOLLOW|LOCKLEAF|KERNELPATH, UIO_SYSSPACE, name, p);
>  	nid.ni_pledge = PLEDGE_RPATH;
>  
> -	KERNEL_LOCK();
>  	error = namei(&nid);
>  	pool_put(&namei_pool, name);
>  	if (error)
>