Date: Mon, 25 Sep 2000 13:28:03 -0400 (EDT) From: Bosko Milekic <bmilekic@technokratis.com> To: Alfred Perlstein <bright@wintelcom.net> Cc: arch@FreeBSD.ORG, cp@FreeBSD.ORG Subject: Re: need advice, fsetown annoyances and mpsafeness. Message-ID: <Pine.BSF.4.21.0009251314260.15801-100000@jehovah.technokratis.com> In-Reply-To: <20000924125311.Q9141@fw.wintelcom.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0009251314260.15801-100000>