wlr_seat: Fix edge cases with serial validation

include/wlr/types/wlr_seat.h

@@ -21,25 +21,12 @@ struct wlr_serial_range {
 	uint32_t min_incl;
 	uint32_t max_incl;
 };
-
 struct wlr_serial_ringset {
 	struct wlr_serial_range data[WLR_SERIAL_RINGSET_SIZE];
 	int end;
 	int count;
 };
 
-/**
- * Add a new serial number to the set. The number must be larger than
- * all other values already added
- */
-void wlr_serial_add(struct wlr_serial_ringset *set, uint32_t serial);
-
-/**
- * Return false if the serial number is definitely not in the set, true
- * otherwise.
- */
-bool wlr_serial_maybe_valid(struct wlr_serial_ringset *set, uint32_t serial);
-
 /**
  * Contains state for a single client's bound wl_seat resource and can be used
  * to issue input events to that client. The lifetime of these objects is
@@ -62,6 +49,7 @@ struct wlr_seat_client {
 	} events;
 
 	// set of serials which were sent to the client on this seat
+	// for use by wlr_seat_client_{next_serial,validate_event_serial}
 	struct wlr_serial_ringset serials;
 };
 
@@ -652,10 +640,24 @@ bool wlr_seat_validate_touch_grab_serial(struct wlr_seat *seat,
 /**
  * Return a new serial (from wl_display_serial_next()) for the client, and
  * update the seat client's set of valid serials. Use this for all input
- * events.
+ * events; otherwise wlr_seat_client_validate_event_serial() may fail when
+ * handed a correctly functioning client's request serials.
  */
 uint32_t wlr_seat_client_next_serial(struct wlr_seat_client *client);
 
+/**
+ * Return true if the serial number could have been produced by
+ * wlr_seat_client_next_serial() and is "older" (by less than UINT32_MAX/2) than
+ * the current display serial value.
+ *
+ * This function should have no false negatives, and the only false positive
+ * responses allowed are for elements that are still "older" than the current
+ * display serial value and also older than all serial values remaining in
+ * the seat client's serial ring buffer, if that buffer is also full.
+ */
+bool wlr_seat_client_validate_event_serial(struct wlr_seat_client *client,
+	uint32_t serial);
+
 /**
  * Get a seat client from a seat resource. Returns NULL if inert.
  */

types/data_device/wlr_data_device.c

@@ -144,7 +144,7 @@ void seat_client_send_selection(struct wlr_seat_client *seat_client) {
 void wlr_seat_request_set_selection(struct wlr_seat *seat,
 		struct wlr_seat_client *client,
 		struct wlr_data_source *source, uint32_t serial) {
-	if (client && !wlr_serial_maybe_valid(&client->serials, serial)) {
+	if (client && !wlr_seat_client_validate_event_serial(client, serial)) {
 		wlr_log(WLR_DEBUG, "Rejecting set_selection request, "
 			"serial %"PRIu32" was never given to client", serial);
 		return;

types/seat/wlr_seat.c

@@ -142,6 +142,9 @@ static void seat_handle_bind(struct wl_client *client, void *_wlr_seat,
 		wl_signal_init(&seat_client->events.destroy);
 
 		wl_list_insert(&wlr_seat->clients, &seat_client->link);
+
+		// ensure first entry will have index zero
+		seat_client->serials.end = WLR_SERIAL_RINGSET_SIZE - 1;
 	}
 
 	wl_resource_set_implementation(wl_resource, &seat_impl,
@@ -371,11 +374,13 @@ bool wlr_seat_validate_grab_serial(struct wlr_seat *seat, uint32_t serial) {
 	return true;
 }
 
-void wlr_serial_add(struct wlr_serial_ringset *set, uint32_t serial) {
+uint32_t wlr_seat_client_next_serial(struct wlr_seat_client *client) {
+	uint32_t serial = wl_display_next_serial(wl_client_get_display(client->client));
+	struct wlr_serial_ringset *set = &client->serials;
+
 	if (set->count == 0 || set->data[set->end].max_incl + 1 != serial) {
+		if (set->count < WLR_SERIAL_RINGSET_SIZE) {
 			set->count++;
-		if (set->count > WLR_SERIAL_RINGSET_SIZE) {
-			set->count = WLR_SERIAL_RINGSET_SIZE;
 		}
 		set->end = (set->end + 1) % WLR_SERIAL_RINGSET_SIZE;
 		set->data[set->end].min_incl = serial;
@@ -383,23 +388,32 @@ void wlr_serial_add(struct wlr_serial_ringset *set, uint32_t serial) {
 	} else {
 		set->data[set->end].max_incl = serial;
 	}
-}
 
-bool wlr_serial_maybe_valid(struct wlr_serial_ringset *set, uint32_t serial) {
-	for (int i = 0; i < set->count; i++) {
-		int j = (set->end - i + WLR_SERIAL_RINGSET_SIZE) % WLR_SERIAL_RINGSET_SIZE;
-		if (set->data[j].max_incl - serial > UINT32_MAX / 2) {
-			return false;
-		}
-		if (serial - set->data[j].min_incl <= UINT32_MAX / 2) {
-			return true;
-		}
-	}
-	return true;
-}
-
-uint32_t wlr_seat_client_next_serial(struct wlr_seat_client *client) {
-	uint32_t serial = wl_display_next_serial(wl_client_get_display(client->client));
-	wlr_serial_add(&client->serials, serial);
 	return serial;
 }
+
+bool wlr_seat_client_validate_event_serial(struct wlr_seat_client *client, uint32_t serial) {
+	uint32_t cur = wl_display_get_serial(wl_client_get_display(client->client));
+	struct wlr_serial_ringset *set = &client->serials;
+	uint32_t rev_dist = cur - serial;
+
+	if (rev_dist >= UINT32_MAX / 2) {
+		// serial is closer to being 'newer' instead of 'older' than
+		// the current serial, so it's either invalid or incredibly old
+		return false;
+	}
+
+	for (int i = 0; i < set->count; i++) {
+		int j = (set->end - i + WLR_SERIAL_RINGSET_SIZE) % WLR_SERIAL_RINGSET_SIZE;
+		if (rev_dist < cur - set->data[j].max_incl) {
+			return false;
+		}
+		if (rev_dist <= cur - set->data[j].min_incl) {
+			return true;
+		}
+	}
+
+	// Iff the set is full, then `rev_dist` is large enough that serial
+	// could already have been recycled out of the set.
+	return set->count == WLR_SERIAL_RINGSET_SIZE;
+}

types/wlr_primary_selection.c

@@ -44,7 +44,7 @@ void wlr_primary_selection_source_send(
 void wlr_seat_request_set_primary_selection(struct wlr_seat *seat,
 		struct wlr_seat_client *client,
 		struct wlr_primary_selection_source *source, uint32_t serial) {
-	if (client && !wlr_serial_maybe_valid(&client->serials, serial)) {
+	if (client && !wlr_seat_client_validate_event_serial(client, serial)) {
 		wlr_log(WLR_DEBUG, "Rejecting set_primary_selection request, "
 			"serial %"PRIu32" was never given to client", serial);
 		return;