v7 shared text bug & fix.

[Dave Buxbaum]

I recently found an interesting bug in the Version 7 shared text code
which, under the right circumstances, causes deadlock in the kernel.

The initial symptom was that our V7 system would "hang" occasionally while
under heavy disk load. (This would happen in anywhere from  2 to 48
hours, while running a test which was really disk bound).

The deadlock results from the following situation:

Consider N number of processes executing out of a shared text segment.
At some point one of the processes executes the exit system call, while
the others remain in various states of completion.  From within the
exit(), a call is made to xfree() to relinquish use of the shared text
segment:

/*
 */
xfree()
{
	register struct text *xp;
	register struct inode *ip;

	if((xp=u.u_procp->p_textp) == NULL)
		return;
	dprelse(u.u_procp);
	xlock(xp);
	xp->x_flag &= ~XLOCK;
	u.u_procp->p_textp = NULL;       	/* TROUBLE BREWING */
	u.u_ptsize = 0;
	ip = xp->x_iptr;
	if(--xp->x_count==0 && (ip->i_mode&ISVTX)==0) {
		xp->x_iptr = NULL;
		mfree(swapmap, ctod(xp->x_size), xp->x_daddr);
		mfree(coremap, xp->x_size, xp->x_caddr);
		ip->i_flag &= ~ITEXT;
		if (ip->i_flag&ILOCK)
			ip->i_count--;
		else
			iput(ip);
	} else
		xccdec(xp);
}

Note that the text pointer in the user structure is set NULL, thus
severing the connection between the text segment and the exiting
process. The interesting case occurs when the else clause is taken; a
call to xccdec().  Here is xccdec():

xccdec(xp)
register struct text *xp;
{

	if (xp==NULL || xp->x_ccount==0)
		return;
	xlock(xp);
	if (--xp->x_ccount==0) {
		if (xp->x_flag&XWRIT) {
			xp->x_flag &= ~XWRIT;
			swap(xp->x_daddr,xp->x_caddr,xp->x_size,B_WRITE);
		}
		mfree(coremap, xp->x_size, xp->x_caddr);
	}
	xunlock(xp);
}

The lock is granted after the text pointer has been set NULL. If the
call to swap causes a context switch, which it can under certain
circumstances, we find the dying process out on the swap device while
holding the text lock granted in xccdec().

The scheduler does check that the given process is not holding a lock
on a text segment, but in this case the connection between the process
table and text table has already been severed. This renders the check
useless.

The next time the scheduler selects a candidate who wants to run from
the same segment we have big trouble:

sched()
{
	.
	.
	.
	/* Look for a candidate */
	.
	.
	.
	/*
	 * If there is no one there, wait.
	 */
	if (outage == -20000) {
		runout++;
		sleep((caddr_t)&runout, PSWP);
		goto loop;
	}

	/*
	 * Found one, see if there is core for that process;
	 * if so, swap it in.
	 */

	if (swapin(p))
	.
	.
	/* If not, swap others out until we have room */
	.
	.
	for (rp = &proc[0]; rp < &proc[NPROC]; rp++) {
		if (rp->p_stat==SZOMB
		 || (rp->p_flag&(SSYS|SLOCK|SULOCK|SLOAD))!=SLOAD)
			continue;

		/*** HERE IS THE CHECK ***/

		if (rp->p_textp && rp->p_textp->x_flag&XLOCK)  
			continue;

The first thing swapin() does if the new process is running a shared
text segment is increment the incore count. Before doing this, a call to
xlock() is made and since the swapped out dying process is still
holding the lock, the kernel sleeps in sched() waiting for a lock it
will never get.

The fix I applied is rather simple and, there is more than one solution.
I chose to lock the dying process in core while in this "critical section".

xccdec(xp)
register struct text *xp;
{
	register int	prevlock;

	if (xp==NULL || xp->x_ccount==0)
		return;
	xlock(xp);
	if (!(prevlock = u.u_procp->p_flag & SLOCK))	/* Bug fix:	*/
		u.u_procp->p_flag |= SLOCK;	/*    Don't let this process  */
	if (--xp->x_ccount==0) {		/*    swap out while holding  */
		if (xp->x_flag&XWRIT) {		/*    the text lock.	      */
			xp->x_flag &= ~XWRIT;
			swap(xp->x_daddr,xp->x_caddr,xp->x_size,B_WRITE);
		}
		mfree(coremap, xp->x_size, xp->x_caddr);
	}
	xunlock(xp);
	if (!prevlock)
		u.u_procp->p_flag &= ~SLOCK;	/* 	All done.	    */
}

Any comments on this bug fix?

	David Buxbaum

	dual!bux at BERKELEY.ARPA
	{ihnp4,ucbvax,hplabs,decwrl,cbosgd,sun,nsc,apple,pyramid}!dual!bux
	Dual Systems Corporation, Berkeley, California