Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: fstat sysctl crashes kernel
To:
Vitaliy Makkoveev <mvs@openbsd.org>
Cc:
Mark Kettenis <mark.kettenis@xs4all.nl>, tech@openbsd.org
Date:
Sat, 10 Aug 2024 10:57:35 +0200

Download raw body.

Thread
  • Vitaliy Makkoveev:

    fstat sysctl crashes kernel

  • Vitaliy Makkoveev:

    fstat sysctl crashes kernel

  • On Fri, Aug 09, 2024 at 10:07:42PM +0300, Vitaliy Makkoveev wrote:
    > i386 is stable running dpb(1) on NFS with many fstat(1) loops more
    > than 4 hours.
    
    My stress test is running for 18 hours now.  Also diff survived a
    full regress test.
    
    Kernel lock and not exiting the process while sysctl sleeps is all
    we need.  Thanks for fixing.
    
    OK bluhm@
    
    > > My previous similar diffs were failed in NFS cases, so it makes sense to
    > > drop non NFS setups.
    > > 
    > > Index: sys/kern/kern_exit.c
    > > ===================================================================
    > > RCS file: /cvs/src/sys/kern/kern_exit.c,v
    > > diff -u -p -r1.230 kern_exit.c
    > > --- sys/kern/kern_exit.c	6 Aug 2024 18:41:20 -0000	1.230
    > > +++ sys/kern/kern_exit.c	9 Aug 2024 11:51:30 -0000
    > > @@ -151,6 +151,9 @@ exit1(struct proc *p, int xexit, int xsi
    > > 			    PS_ISPWAIT);
    > > 			wakeup(pr->ps_pptr);
    > > 		}
    > > +
    > > +		/* Wait for concurrent `allprocess' loops */
    > > +		refcnt_finalize(&pr->ps_refcnt, "psdtor");
    > > 	}
    > > 
    > > 	/* unlink ourselves from the active threads */
    > > Index: sys/kern/kern_fork.c
    > > ===================================================================
    > > RCS file: /cvs/src/sys/kern/kern_fork.c,v
    > > diff -u -p -r1.261 kern_fork.c
    > > --- sys/kern/kern_fork.c	6 Aug 2024 08:44:54 -0000	1.261
    > > +++ sys/kern/kern_fork.c	9 Aug 2024 11:51:30 -0000
    > > @@ -178,6 +178,8 @@ thread_new(struct proc *parent, vaddr_t 
    > > void
    > > process_initialize(struct process *pr, struct proc *p)
    > > {
    > > +	refcnt_init(&pr->ps_refcnt);
    > > +
    > > 	/* initialize the thread links */
    > > 	pr->ps_mainproc = p;
    > > 	TAILQ_INIT(&pr->ps_threads);
    > > Index: sys/kern/kern_sysctl.c
    > > ===================================================================
    > > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
    > > diff -u -p -r1.436 kern_sysctl.c
    > > --- sys/kern/kern_sysctl.c	8 Aug 2024 15:02:36 -0000	1.436
    > > +++ sys/kern/kern_sysctl.c	9 Aug 2024 11:51:30 -0000
    > > @@ -1686,6 +1686,9 @@ sysctl_file(int *name, u_int namelen, ch
    > > 				/* not the pid we are looking for */
    > > 				continue;
    > > 			}
    > > +
    > > +			refcnt_take(&pr->ps_refcnt);
    > > +
    > > 			matched = 1;
    > > 			fdp = pr->ps_fd;
    > > 			if (pr->ps_textvp)
    > > @@ -1702,6 +1705,9 @@ sysctl_file(int *name, u_int namelen, ch
    > > 				FILLIT(fp, fdp, i, NULL, pr);
    > > 				FRELE(fp, p);
    > > 			}
    > > +
    > > +			refcnt_rele_wake(&pr->ps_refcnt);
    > > +
    > > 			/* pid is unique, stop searching */
    > > 			if (arg >= 0)
    > > 				break;
    > > @@ -1721,6 +1727,9 @@ sysctl_file(int *name, u_int namelen, ch
    > > 				/* not the uid we are looking for */
    > > 				continue;
    > > 			}
    > > +
    > > +			refcnt_take(&pr->ps_refcnt);
    > > +
    > > 			fdp = pr->ps_fd;
    > > 			if (fdp->fd_cdir)
    > > 				FILLIT(NULL, NULL, KERN_FILE_CDIR, fdp->fd_cdir, pr);
    > > @@ -1734,6 +1743,8 @@ sysctl_file(int *name, u_int namelen, ch
    > > 				FILLIT(fp, fdp, i, NULL, pr);
    > > 				FRELE(fp, p);
    > > 			}
    > > +
    > > +			refcnt_rele_wake(&pr->ps_refcnt);
    > > 		}
    > > 		break;
    > > 	default:
    > > Index: sys/sys/proc.h
    > > ===================================================================
    > > RCS file: /cvs/src/sys/sys/proc.h,v
    > > diff -u -p -r1.367 proc.h
    > > --- sys/sys/proc.h	6 Aug 2024 08:44:54 -0000	1.367
    > > +++ sys/sys/proc.h	9 Aug 2024 11:51:30 -0000
    > > @@ -142,6 +142,8 @@ struct pinsyscall {
    > >  *	T	itimer_mtx
    > >  */
    > > struct process {
    > > +	struct refcnt ps_refcnt;
    > > +
    > > 	/*
    > > 	 * ps_mainproc is the original thread in the process.
    > > 	 * It's only still special for the handling of
    > > 
    
    
    
  • Vitaliy Makkoveev:

    fstat sysctl crashes kernel

  • Vitaliy Makkoveev:

    fstat sysctl crashes kernel