Date: Thu, 8 Mar 2007 09:42:46 -0500 From: Ed Maste <emaste@phaedrus.sandvine.ca> To: Divacky Roman <xdivac02@stud.fit.vutbr.cz> Cc: freebsd-hackers@freebsd.org Subject: Re: Hung kernel from sysv semaphore semu_list corruption Message-ID: <20070308144245.GA69780@sandvine.com> In-Reply-To: <20070308095522.GA14973@stud.fit.vutbr.cz> References: <20070307230731.GA71684@sandvine.com> <20070308095522.GA14973@stud.fit.vutbr.cz>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Mar 08, 2007 at 10:55:22AM +0100, Divacky Roman wrote:
> this is wrong.. you cannot remove element from a *LIST when its iterated using *LIST_FOREACH.
> Use *LIST_FOREACH_SAFE instead...
We're not freeing the item in the loop so it would work unless
QUEUE_MACRO_DEBUG is turned on to intentionally trash the link
pointer on _REMOVE. You're right though it's not the correct approach.
There's an efficiency issue with the previous patch anyhow in that
SLIST_REMOVE walks the list again to find the element to remove.
How about the patch below instead:
Index: sysv_sem.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/sysv_sem.c,v
retrieving revision 1.85
diff -u -r1.85 sysv_sem.c
--- sysv_sem.c 22 Oct 2006 11:52:13 -0000 1.85
+++ sysv_sem.c 8 Mar 2007 14:22:43 -0000
@@ -1301,8 +1301,10 @@
*/
SEMUNDO_LOCK();
SLIST_FOREACH_PREVPTR(suptr, supptr, &semu_list, un_next) {
- if (suptr->un_proc == p)
+ if (suptr->un_proc == p) {
+ *supptr = SLIST_NEXT(suptr, un_next);
break;
+ }
}
SEMUNDO_UNLOCK();
@@ -1362,8 +1364,9 @@
* Deallocate the undo vector.
*/
DPRINTF(("removing vector\n"));
+ SEMUNDO_LOCK();
suptr->un_proc = NULL;
- *supptr = SLIST_NEXT(suptr, un_next);
+ SEMUNDO_UNLOCK();
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070308144245.GA69780>