From owner-freebsd-arch Mon Sep 25 10:24:47 2000 Delivered-To: freebsd-arch@freebsd.org Received: from field.videotron.net (field.videotron.net [205.151.222.108]) by hub.freebsd.org (Postfix) with ESMTP id 84B6137B422; Mon, 25 Sep 2000 10:24:30 -0700 (PDT) Received: from modemcable136.203-201-24.mtl.mc.videotron.ca ([24.201.203.136]) by field.videotron.net (Sun Internet Mail Server sims.3.5.1999.12.14.10.29.p8) with ESMTP id <0G1G00CEDDOINM@field.videotron.net>; Mon, 25 Sep 2000 13:24:19 -0400 (EDT) Date: Mon, 25 Sep 2000 13:28:03 -0400 (EDT) From: Bosko Milekic Subject: Re: need advice, fsetown annoyances and mpsafeness. In-reply-to: <20000924125311.Q9141@fw.wintelcom.net> To: Alfred Perlstein Cc: arch@FreeBSD.ORG, cp@FreeBSD.ORG Message-id: MIME-version: 1.0 Content-type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sun, 24 Sep 2000, Alfred Perlstein wrote: > It's really a lot more evil than you think. Yeah, I noticed after sending the Email. > The race is in the object (socket/tty) checking the pointer and > then dereferencing it. > > A broken solution is to lock the sigio struct or provide a backreference > to the socket/tty lock, after banging my head against my desk for some time I came across this solution: This looks somewhat like what you mentionned in (2) in your earlier post. The sigio struct will only be freed by the object. I think this is a reasonable solution. > (assuming pfind/pgfind return the proc/pgrp locked) > [...] > /* > * ok this is somewhat tricky, we examine what the sigio is attached > * to, whatever it is proc/pgrp we need to use the search functions > * to ensure atomicity. If we get back ESRCH that's ok, that means > * we lost the race, just free it. > * if we get back a pointer we then need to make sure that the pgid > * hasn't been NULLed out because we lost the race between looking > * at the sigio and locking the proc/pgrp > * (most likely pid/pgid wraparound) > */ > pid = sigio->sio_pgid; > > if (pid < 0) { > struct pgrp *p; > > if ((pgrp = pgfind(pid)) != NULL) { > /* funsetown_proc would have set this to zero */ > if (sigio->sio_pgid != 0) > SLIST_REMOVE(&sigio->sio_pgrp->pg_sigiolst, sigio, > sigio, sio_pgsigio); > PGRP_UNLOCK(&sigio->sio_pgrp); > } > } else if (pid > 0) { > struct proc *p; > > if ((p = pfind(pid)) != NULL) { > /* funsetown_proc would have set this to zero */ > if (sigio->sio_pgid != 0) > SLIST_REMOVE(&sigio->sio_proc->p_sigiolst, sigio, > sigio, sio_pgsigio); > PROC_UNLOCK(&sigio->sio_proc); > } > } > > out: > crfree(sigio->sio_ucred); > FREE(sigio, M_SIGIO); > } Looks good. > /* > * NULL out a sigio struct attached to a process/pgrp > * must be called with the object (struct proc/pgrp) locked > * this is to be called from the perspective of the process/pgrp > * > * called from the proc/pgid at teardown > * proc/pgid must be locked > */ > void > funsetown_proc(sigio) > struct sigio *sigio; > { > int s; > > if (sigio == NULL) > return; > if (sigio->sio_pgid < 0) { > SLIST_REMOVE(&sigio->sio_pgrp->pg_sigiolst, sigio, > sigio, sio_pgsigio); > } else /* if ((*sigiop)->sio_pgid > 0) */ { > SLIST_REMOVE(&sigio->sio_proc->p_sigiolst, sigio, > sigio, sio_pgsigio); > } > sigio->sio_pgid = 0; > } > > /* > * Free a list of sigio structures. > * > * called from the proc/pgid at teardown > * proc/pgid must be locked > */ > void > funsetownlst(sigiolst) > struct sigiolst *sigiolst; > { > struct sigio *sigio; > > while ((sigio = SLIST_FIRST(sigiolst)) != NULL) > funsetown(sigio); > } > > > Questions? Comments? Question: You don't seem to be protecting the actual sigiolst list with a lock. What happens if you've got two different processes manipulating the list? Each one may be locked, but regardless, your list can still be trashed. > -- > -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] > "I have the heart of a child; I keep it in a jar on my desk." Regards, Bosko Milekic bmilekic@technokratis.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message