From: Imre Vadász <imre@vdsz.com>
Date: Sat, 19 Aug 2017 19:50:28 +0000 (+0200)
Subject: kqueue: Make EVFILT_USER event behaviour more consistent.
X-Git-Tag: v5.1.0~207
X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/cf83cc1901df816457f1aae6fa950a10fc75754b

kqueue: Make EVFILT_USER event behaviour more consistent.

* Stop abusing the kn->kn_sfflags value for storing the current state of
  the EVFILT_USER filter. Instead use kn->kn_fflags like other filters.
  Similarly store the data value in kn->kn_data instead of kn->kn_sdata.
  This means that the fflags value gets reset when EV_CLEAR was specified
  when adding the event, and the event is received by userspace. This
  behaviour is consistent with existing kqueue filters, and allows using
  EVFILT_USER properly as an edge-triggered event when using the fflags,
  and not just level-triggered.

* Don't clear kn->kn_fflags when the event is modified with EV_CLEAR. Doing
  this wasn't affecting the actual state of the EVFILT_USER event before
  this change (since the state was kept in kn->kn_sfflags instead).

* All this also avoids blindly copying the fflags value that was specified
  when adding the event. Instead the NOTE_FFLAGSMASK mask is applied, and
  the NOTE_FF* options are used, so the returned fflags value should now
  always only have the lower 24 bits set.

* Make setting the fflags and data value when adding the event work as
  might be expected.
---

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index cdd44713e5..e776ce55ef 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -521,11 +521,39 @@ filt_timer(struct knote *kn, long hint)
 static int
 filt_userattach(struct knote *kn)
 {
+	u_int ffctrl;
+
 	kn->kn_hook = NULL;
 	if (kn->kn_sfflags & NOTE_TRIGGER)
 		kn->kn_ptr.hookid = 1;
 	else
 		kn->kn_ptr.hookid = 0;
+
+	ffctrl = kn->kn_sfflags & NOTE_FFCTRLMASK;
+	kn->kn_sfflags &= NOTE_FFLAGSMASK;
+	switch (ffctrl) {
+	case NOTE_FFNOP:
+		break;
+
+	case NOTE_FFAND:
+		kn->kn_fflags &= kn->kn_sfflags;
+		break;
+
+	case NOTE_FFOR:
+		kn->kn_fflags |= kn->kn_sfflags;
+		break;
+
+	case NOTE_FFCOPY:
+		kn->kn_fflags = kn->kn_sfflags;
+		break;
+
+	default:
+		/* XXX Return error? */
+		break;
+	}
+	/* We just happen to copy this value as well. Undocumented. */
+	kn->kn_data = kn->kn_sdata;
+
 	return 0;
 }
 
@@ -558,22 +586,23 @@ filt_usertouch(struct knote *kn, struct kevent *kev, u_long type)
 			break;
 
 		case NOTE_FFAND:
-			kn->kn_sfflags &= kev->fflags;
+			kn->kn_fflags &= kev->fflags;
 			break;
 
 		case NOTE_FFOR:
-			kn->kn_sfflags |= kev->fflags;
+			kn->kn_fflags |= kev->fflags;
 			break;
 
 		case NOTE_FFCOPY:
-			kn->kn_sfflags = kev->fflags;
+			kn->kn_fflags = kev->fflags;
 			break;
 
 		default:
 			/* XXX Return error? */
 			break;
 		}
-		kn->kn_sdata = kev->data;
+		/* We just happen to copy this value as well. Undocumented. */
+		kn->kn_data = kev->data;
 
 		/*
 		 * This is not the correct use of EV_CLEAR in an event
@@ -586,15 +615,22 @@ filt_usertouch(struct knote *kn, struct kevent *kev, u_long type)
 		 */
 		if (kev->flags & EV_CLEAR) {
 			kn->kn_ptr.hookid = 0;
+			/*
+			 * Clearing kn->kn_data is fine, since it gets set
+			 * every time anyway. We just shouldn't clear
+			 * kn->kn_fflags here, since that would limit the
+			 * possible uses of this API. NOTE_FFAND or
+			 * NOTE_FFCOPY should be used for explicitly clearing
+			 * kn->kn_fflags.
+			 */
 			kn->kn_data = 0;
-			kn->kn_fflags = 0;
 		}
 		break;
 
         case EVENT_PROCESS:
 		*kev = kn->kn_kevent;
-		kev->fflags = kn->kn_sfflags;
-		kev->data = kn->kn_sdata;
+		kev->fflags = kn->kn_fflags;
+		kev->data = kn->kn_data;
 		if (kn->kn_flags & EV_CLEAR) {
 			kn->kn_ptr.hookid = 0;
 			/* kn_data, kn_fflags handled by parent */