[9fans] Potential race in syspipe()

[Alexander Viro <viro@math.psu.edu>]

	Umm... Unless I'm seriously misreading the code, there is a race
between syspipe() and sysdup(). Scenario:

CPU1 (in syspipe()):
        c[0] = d->open(c[0], ORDWR);
        c[1] = d->open(c[1], ORDWR);
        fd[0] = newfd(c[0]);
/* got so far */
        fd[1] = newfd(c[1]);
/* failed on that one for whatever reason */

CPU2 (in sysdup()):
		...
                lock(f);
		...
                oc = f->fd[fd];
                f->fd[fd] = c;
                unlock(f);
                if(oc)
                        cclose(oc);
/* where fd happens to be equal to fd[0] on the CPU1 */
/* oc == CPU1's c[0], indeed */

CPU1:
        if(fd[0] < 0 || fd[1] < 0)
                error(Enofd);
/* there we go */
                cclose(c[0]);
/* woops - extra cclose on c[0] and we are welcome to... */
        if(c->flag&CFREE)
                panic("cclose %lux", getcallerpc(&c));


Giving user enough rope to hang himself is a good idea, but panic() goes
somewhat beyond "himself", especially when triggered on CPU server...
AFAICS the cleanish solution would be along the lines of

int
newfd2(int fd[2], Chan *c[2])
{
        Fgrp *f;
	int i,j;

        f = up->fgrp;
        lock(f);
        for(i=0; i<f->nfd; i++)
                if(f->fd[i] == 0)
                        break;
	for(j=i+1; j<f->nfd; j++)
		if (f->fd[j] == 0)
			break;
      
        if(j >= f->nfd && growfd(f, j) < 0){
                unlock(f);
                return -1;
        }
        if(j > f->maxfd)
                f->maxfd = j;
        f->fd[fd[0]=i] = c[0];
        f->fd[fd[1]=j] = c[1];
        unlock(f);
        return 0;
}

and using it in syspipe() (instead of two newfd() calls). That way the
error recovery in syspipe() doesn't have to touch anything that might be
anywhere near the ->fd[].

Comments? AFAICS _anything_ that goes into ->fd[] must be treated as final
as soon as you release the lock on ->fgrp (due to the dup2() issues).

							Al