config: Add unique device identifiers (#709 et al)

A common problem is that users will have devices which split their functionality
between several device nodes (e.g laptop keyboards). It is often desirable to
only remap a subset of these device nodes so that some of them can operate
unintercepted by keyd (e.g touchpads) or be assigned to a different config.

This patch moves away from vendor/product id pairs by introducing unique identifiers.
To maintain backward compatibility, prefix matching is used and the old identifiers
are valid substrings of the new ones.

As a byproduct of this, a new bug/feature is born. <vendor id> in
isolation will now also match any device with the given vendor id.
Woe betide the user that makes use of this.

Makefile

@@ -38,7 +38,7 @@ all:
 	sed -e 's#@PREFIX@#$(PREFIX)#' src/vkbd/usb-gadget.service.in > src/vkbd/usb-gadget.service
 	$(CC) $(CFLAGS) -O3 $(COMPAT_FILES) src/*.c src/vkbd/$(VKBD).c -lpthread -o bin/keyd $(LDFLAGS)
 debug:
-	CFLAGS="-g -Wunused" $(MAKE)
+	CFLAGS="-g -fsanitize=address -Wunused" $(MAKE)
 compose:
 	-mkdir data
 	./scripts/generate_xcompose

docs/keyd.scdoc

@@ -75,8 +75,8 @@ section that has one of the following forms:
 ```
 	[ids]
 
-	<vendor id 1>:<product id 1>
-	<vendor id 2>:<product id 2>
+	<id 1 (obtained via keyd monitor)>
+	<id 2>
 	...
 ```
 
@@ -86,8 +86,8 @@ or
 	[ids]
 
 	*
-	-<vendor id 1>:<product id 1>
-	-<vendor id 2>:<product id 2>
+	-<id 1>
+	-<id 2>
 	...
 ```
 
@@ -107,12 +107,12 @@ Will match all devices which *do not*(2) have the id _0123:4567_, while:
 
 ```
 	[ids]
+
 	0123:4567
 ```
 
-will exclusively match any devices which do. Note that you can obtain
-the '<vendor id>:<product id>' specifiers from the monitor command (see
-_COMMANDS_).
+will exclusively match any devices which do. Device ids can be obtained from
+the monitor command (see _COMMANDS_).
 
 Each subsequent section of the file corresponds to a _layer_ (with the exception
 of _[global]_ (see _GLOBALS_).
@@ -120,7 +120,7 @@ of _[global]_ (see _GLOBALS_).
 Config errors will appear in the log output and can be accessed in the usual
 way using your system's service manager (e.g sudo journalctl -eu keyd).
 
-If a vendor/product pair matches more than one device type, the prefix k: may
+If an id matches more than one device type, the prefix k: may
 be used to exclusively match keyboards and the prefix m: may be used to
 exclusively match mice. (E.g m:046d:b01d)
 

src/config.c

@@ -773,39 +773,28 @@ static void parse_id_section(struct config *config, struct ini_section *section)
 
 		if (!strcmp(s, "*")) {
 			config->wildcard = 1;
+		} else if (strstr(s, "m:") == s) {
+			assert(config->nr_ids < ARRAY_SIZE(config->ids));
+			config->ids[config->nr_ids].flags = ID_MOUSE;
+
+			snprintf(config->ids[config->nr_ids++].id, sizeof(config->ids[0].id), "%s", s+2);
+		} else if (strstr(s, "k:") == s) {
+			assert(config->nr_ids < ARRAY_SIZE(config->ids));
+			config->ids[config->nr_ids].flags = ID_KEYBOARD;
+
+			snprintf(config->ids[config->nr_ids++].id, sizeof(config->ids[0].id), "%s", s+2);
+		} else if (strstr(s, "-") == s) {
+			assert(config->nr_ids < ARRAY_SIZE(config->ids));
+			config->ids[config->nr_ids].flags = ID_EXCLUDED;
+
+			snprintf(config->ids[config->nr_ids++].id, sizeof(config->ids[0].id), "%s", s+1);
+		} else if (strlen(s) < sizeof(config->ids[config->nr_ids].id)-1) {
+			assert(config->nr_ids < ARRAY_SIZE(config->ids));
+			config->ids[config->nr_ids].flags = ID_KEYBOARD | ID_MOUSE;
+
+			snprintf(config->ids[config->nr_ids++].id, sizeof(config->ids[0].id), "%s", s);
 		} else {
-			if (sscanf(s, "m:%hx:%hx", &vendor, &product) == 2) {
-				assert(config->nr_ids < ARRAY_SIZE(config->ids));
-				config->ids[config->nr_ids].product = product;
-				config->ids[config->nr_ids].vendor = vendor;
-				config->ids[config->nr_ids].flags = ID_MOUSE;
-
-				config->nr_ids++;
-			} else if (sscanf(s, "k:%hx:%hx", &vendor, &product) == 2) {
-				assert(config->nr_ids < ARRAY_SIZE(config->ids));
-				config->ids[config->nr_ids].product = product;
-				config->ids[config->nr_ids].vendor = vendor;
-				config->ids[config->nr_ids].flags = ID_KEYBOARD;
-
-				config->nr_ids++;
-			} else if (sscanf(s, "-%hx:%hx", &vendor, &product) == 2) {
-				assert(config->nr_ids < ARRAY_SIZE(config->ids));
-				config->ids[config->nr_ids].product = product;
-				config->ids[config->nr_ids].vendor = vendor;
-				config->ids[config->nr_ids].flags = ID_EXCLUDED;
-
-				config->nr_ids++;
-			} else if (sscanf(s, "%hx:%hx", &vendor, &product) == 2) {
-				assert(config->nr_ids < ARRAY_SIZE(config->ids));
-				config->ids[config->nr_ids].product = product;
-				config->ids[config->nr_ids].vendor = vendor;
-				config->ids[config->nr_ids].flags = ID_KEYBOARD | ID_MOUSE;
-
-				config->nr_ids++;
-			}
-			else {
-				warn("%s is not a valid device id", s);
-			}
+			warn("%s is not a valid device id", s);
 		}
 	}
 }
@@ -958,12 +947,13 @@ int config_parse(struct config *config, const char *path)
 	return config_parse_string(config, content);
 }
 
-int config_check_match(struct config *config, uint16_t vendor, uint16_t product, uint8_t flags)
+int config_check_match(struct config *config, const char *id, uint8_t flags)
 {
 	size_t i;
 
 	for (i = 0; i < config->nr_ids; i++) {
-		if (config->ids[i].product == product && config->ids[i].vendor == vendor) {
+		//Prefix match to allow matching <product>:<vendor> for backward compatibility.
+		if (strstr(id, config->ids[i].id) == id) {
 			if (config->ids[i].flags & ID_EXCLUDED) {
 				return 0;
 			} else if (config->ids[i].flags & flags) {

src/config.h

@@ -120,8 +120,7 @@ struct config {
 
 	uint8_t wildcard;
 	struct {
-		uint16_t product;
-		uint16_t vendor;
+		char id[64];
 		uint8_t flags;
 	} ids[64];
 
@@ -152,6 +151,6 @@ int config_parse(struct config *config, const char *path);
 int config_add_entry(struct config *config, const char *exp);
 int config_get_layer_index(const struct config *config, const char *name);
 
-int config_check_match(struct config *config, uint16_t vendor, uint16_t product, uint8_t flags);
+int config_check_match(struct config *config, const char *id, uint8_t flags);
 
 #endif

src/daemon.c

@@ -182,16 +182,14 @@ static void load_configs()
 	closedir(dh);
 }
 
-static struct config_ent *lookup_config_ent(uint16_t vendor,
-					    uint16_t product,
-					    uint8_t flags)
+static struct config_ent *lookup_config_ent(const char *id, uint8_t flags)
 {
 	struct config_ent *ent = configs;
 	struct config_ent *match = NULL;
 	int rank = 0;
 
 	while (ent) {
-		int r = config_check_match(&ent->config, vendor, product, flags);
+		int r = config_check_match(&ent->config, id, flags);
 
 		if (r > rank) {
 			match = ent;
@@ -221,24 +219,21 @@ static void manage_device(struct device *dev)
 	if (dev->capabilities & (CAP_MOUSE|CAP_MOUSE_ABS))
 		flags |= ID_MOUSE;
 
-	if ((ent = lookup_config_ent(dev->vendor_id, dev->product_id, flags))) {
+	if ((ent = lookup_config_ent(dev->id, flags))) {
 		if (device_grab(dev)) {
 			keyd_log("DEVICE: y{WARNING} Failed to grab %s\n", dev->path);
 			dev->data = NULL;
 			return;
 		}
 
-		keyd_log("DEVICE: g{match}    %04hx:%04hx  %s\t(%s)\n",
-			  dev->vendor_id, dev->product_id,
-			  ent->config.path,
-			  dev->name);
+		keyd_log("DEVICE: g{match}    %s  %s\t(%s)\n",
+			  dev->id, ent->config.path, dev->name);
 
 		dev->data = ent->kbd;
 	} else {
 		dev->data = NULL;
 		device_ungrab(dev);
-		keyd_log("DEVICE: r{ignoring} %04hx:%04hx  (%s)\n", 
-			  dev->vendor_id, dev->product_id, dev->name);
+		keyd_log("DEVICE: r{ignoring} %s  (%s)\n", dev->id, dev->name);
 	}
 }
 
@@ -523,10 +518,7 @@ static int event_handler(struct event *ev)
 		manage_device(ev->dev);
 		break;
 	case EV_DEV_REMOVE:
-		keyd_log("DEVICE: r{removed}\t%04hx:%04hx %s\n",
-			  ev->dev->vendor_id,
-			  ev->dev->product_id,
-			  ev->dev->name);
+		keyd_log("DEVICE: r{removed}\t%s %s\n", ev->dev->id, ev->dev->name);
 
 		break;
 	case EV_FD_ACTIVITY:

src/device.c

@@ -41,7 +41,7 @@
  * corresponding device should be considered invalid by the caller.
  */
 
-static uint8_t resolve_device_capabilities(int fd)
+static uint8_t resolve_device_capabilities(int fd, int *num_keys, uint8_t *relmask, uint8_t *absmask)
 {
 	const uint32_t keyboard_mask = 1<<KEY_1  | 1<<KEY_2 | 1<<KEY_3 |
 					1<<KEY_4 | 1<<KEY_5 | 1<<KEY_6 |
@@ -50,9 +50,8 @@ static uint8_t resolve_device_capabilities(int fd)
 					1<<KEY_E | 1<<KEY_R | 1<<KEY_T |
 					1<<KEY_Y;
 
+	size_t i;
 	uint32_t mask[BTN_LEFT/32+1] = {0};
-	uint8_t has_rel;
-	uint8_t has_abs;
 	uint8_t capabilities = 0;
 	int has_brightness_key;
 
@@ -61,20 +60,24 @@ static uint8_t resolve_device_capabilities(int fd)
 		return 0;
 	}
 
-	if (ioctl(fd, EVIOCGBIT(EV_REL, 1), &has_rel) < 0) {
+	if (ioctl(fd, EVIOCGBIT(EV_REL, 1), relmask) < 0) {
 		perror("ioctl: ev_rel");
 		return 0;
 	}
 
-	if (ioctl(fd, EVIOCGBIT(EV_ABS, 1), &has_abs) < 0) {
+	if (ioctl(fd, EVIOCGBIT(EV_ABS, 1), absmask) < 0) {
 		perror("ioctl: ev_abs");
 		return 0;
 	}
 
-	if (has_rel || has_abs)
+	*num_keys = 0;
+	for (i = 0; i < sizeof(mask)/sizeof(mask[0]); i++)
+		*num_keys += __builtin_popcount(mask[i]);
+
+	if (*relmask || *absmask)
 		capabilities |= CAP_MOUSE;
 
-	if (has_abs)
+	if (*absmask)
 		capabilities |= CAP_MOUSE_ABS;
 
 	/*
@@ -99,6 +102,9 @@ static int device_init(const char *path, struct device *dev)
 {
 	int fd;
 	int capabilities;
+	int num_keys;
+	uint8_t relmask;
+	uint8_t absmask;
 	struct input_absinfo absinfo;
 
 	if ((fd = open(path, O_RDWR | O_NONBLOCK | O_CLOEXEC, 0600)) < 0) {
@@ -106,7 +112,7 @@ static int device_init(const char *path, struct device *dev)
 		return -1;
 	}
 
-	capabilities = resolve_device_capabilities(fd);
+	capabilities = resolve_device_capabilities(fd, &num_keys, &relmask, &absmask);
 
 	if (ioctl(fd, EVIOCGNAME(sizeof(dev->name)), dev->name) == -1) {
 		keyd_log("ERROR: could not fetch device name of %s\n", dev->path);
@@ -144,10 +150,18 @@ static int device_init(const char *path, struct device *dev)
 		strncpy(dev->path, path, sizeof(dev->path)-1);
 		dev->path[sizeof(dev->path)-1] = 0;
 
+		/*
+		 * Attempt to generate a reproducible unique identifier for each device.
+		 * The product and vendor ids are insufficient to identify some devices since
+		 * they can create multiple device nodes with different capabilities. Thus
+		 * we factor in the capabilities of the resultant evdev node to
+		 * further distinguish between input devices. These should be
+		 * regarded as opaque identifiers by the user.
+		 */
+		snprintf(dev->id, sizeof dev->id, "%04x:%04x:%04x%02x%02x", info.vendor, info.product, num_keys, absmask, relmask);
+
 		dev->fd = fd;
 		dev->capabilities = capabilities;
-		dev->vendor_id = info.vendor;
-		dev->product_id = info.product;
 		dev->data = NULL;
 		dev->grabbed = 0;
 

src/device.h

@@ -23,10 +23,9 @@ struct device {
 
 	uint8_t grabbed;
 	uint8_t capabilities;
-	uint16_t product_id;
-	uint16_t vendor_id;
 	uint8_t is_virtual;
 
+	char id[64];
 	char name[64];
 	char path[256];
 

src/monitor.c

@@ -38,14 +38,12 @@ int event_handler(struct event *ev)
 	const char *name;
 
 	case EV_DEV_ADD:
-		keyd_log("device added: %04x:%04x %s (%s)\n",
-			  ev->dev->vendor_id, ev->dev->product_id,
-			  ev->dev->name, ev->dev->path);
+		keyd_log("device added: %s %s (%s)\n",
+			  ev->dev->id, ev->dev->name, ev->dev->path);
 		break;
 	case EV_DEV_REMOVE:
-		keyd_log("device removed: %04x:%04x %s (%s)\n",
-			  ev->dev->vendor_id, ev->dev->product_id,
-			  ev->dev->name, ev->dev->path);
+		keyd_log("device removed: %s %s (%s)\n",
+			  ev->dev->id, ev->dev->name, ev->dev->path);
 		break;
 	case EV_DEV_EVENT:
 		switch (ev->devev->type) {
@@ -55,11 +53,9 @@ int event_handler(struct event *ev)
 			if (time_flag)
 				keyd_log("r{+%ld} ms\t", ev->timestamp - last_time);
 
-			keyd_log("%s\t%04x:%04x\t%s %s\n",
-				 ev->dev->name,
-				 ev->dev->vendor_id,
-				 ev->dev->product_id, name,
-				 ev->devev->pressed ? "down" : "up");
+			keyd_log("%s\t%s\t%s %s\n",
+				 ev->dev->name, ev->dev->id,
+				 name, ev->devev->pressed ? "down" : "up");
 
 			break;
 		default: