Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Sep 2015 14:05:30 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r287366 - head/sys/kern
Message-ID:  <201509011405.t81E5U0g025928@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Tue Sep  1 14:05:29 2015
New Revision: 287366
URL: https://svnweb.freebsd.org/changeset/base/287366

Log:
  Exit notification for EVFILT_PROC removes knote from the knlist.  In
  particular, this invalidates the knote kn_link linkage, making the
  SLIST_FOREACH() loop accessing undefined values (e.g. trashed by
  QUEUE_MACRO_DEBUG).  If the knote is freed by other thread when kq
  lock is released or when influx is cleared, e.g. by knote_scan() for
  kqueue owning the knote, the iteration step would access freed memory.
  
  Use SLIST_FOREACH_SAFE() to fix iteration.
  
  Diagnosed by:	avg
  Tested by:	avg, lstewart, pawel
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks

Modified:
  head/sys/kern/kern_event.c

Modified: head/sys/kern/kern_event.c
==============================================================================
--- head/sys/kern/kern_event.c	Tue Sep  1 13:51:07 2015	(r287365)
+++ head/sys/kern/kern_event.c	Tue Sep  1 14:05:29 2015	(r287366)
@@ -1930,7 +1930,7 @@ void
 knote(struct knlist *list, long hint, int lockflags)
 {
 	struct kqueue *kq;
-	struct knote *kn;
+	struct knote *kn, *tkn;
 	int error;
 
 	if (list == NULL)
@@ -1942,14 +1942,13 @@ knote(struct knlist *list, long hint, in
 		list->kl_lock(list->kl_lockarg); 
 
 	/*
-	 * If we unlock the list lock (and set KN_INFLUX), we can eliminate
-	 * the kqueue scheduling, but this will introduce four
-	 * lock/unlock's for each knote to test.  If we do, continue to use
-	 * SLIST_FOREACH, SLIST_FOREACH_SAFE is not safe in our case, it is
-	 * only safe if you want to remove the current item, which we are
-	 * not doing.
+	 * If we unlock the list lock (and set KN_INFLUX), we can
+	 * eliminate the kqueue scheduling, but this will introduce
+	 * four lock/unlock's for each knote to test.  Also, marker
+	 * would be needed to keep iteration position, since filters
+	 * or other threads could remove events.
 	 */
-	SLIST_FOREACH(kn, &list->kl_list, kn_selnext) {
+	SLIST_FOREACH_SAFE(kn, &list->kl_list, kn_selnext, tkn) {
 		kq = kn->kn_kq;
 		KQ_LOCK(kq);
 		if ((kn->kn_status & (KN_INFLUX | KN_SCAN)) == KN_INFLUX) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201509011405.t81E5U0g025928>