Merge pull request #516 from vazub/symlink-fix

keventd: fix dangling and misnamed /dev/input/by-id links

keventd/rules.c

@@ -501,6 +501,33 @@ void rules_free(struct rule_list *list)
 
 /* ----- variable substitution ------------------------------------------- */
 
+/*
+ * Syspath of the parent device selected by the last successful
+ * KERNELS/SUBSYSTEMS/DRIVERS/ATTRS== match of the rule being evaluated.
+ * As in udev, $attr{} and %s{} fall back to this parent when the event
+ * device has no such attribute itself; the stock rules rely on it:
+ *
+ *   ATTRS{bInterfaceNumber}=="?*", SYMLINK+="...-if$attr{bInterfaceNumber}-..."
+ *
+ * Cleared at the start of every rule and again once it has been applied.
+ */
+static char matched_parent[PATH_MAX];
+
+/* Read an attribute for $attr{}/%s{}: the event device first, then the matched parent */
+static int subst_read_attr(const struct uevent *ev, const char *attr,
+			   char *buf, size_t len)
+{
+	char path[PATH_MAX];
+
+	if (ev->devpath && !sysfs_read_attr(ev->devpath, attr, buf, len))
+		return 0;
+	if (!matched_parent[0])
+		return -1;
+
+	snprintf(path, sizeof(path), "%s/%s", matched_parent, attr);
+	return sysfs_read_file(path, buf, len);
+}
+
 static void append_str(char *buf, size_t *pos, size_t bufsz, const char *s)
 {
 	size_t l;
@@ -604,8 +631,7 @@ static void subst_value(const char *src, char *buf, size_t bufsz,
 							al = sizeof(attr) - 1;
 						memcpy(attr, as, al);
 						attr[al] = 0;
-						if (!sysfs_read_attr(ev->devpath, attr,
-								     tmp, sizeof(tmp)))
+						if (!subst_read_attr(ev, attr, tmp, sizeof(tmp)))
 							append_str(buf, &pos, bufsz, tmp);
 						p = ae + 1;
 					} else {
@@ -681,7 +707,7 @@ static void subst_value(const char *src, char *buf, size_t bufsz,
 						al = sizeof(attr) - 1;
 					memcpy(attr, as, al);
 					attr[al] = 0;
-					if (!sysfs_read_attr(ev->devpath, attr, tmp, sizeof(tmp)))
+					if (!subst_read_attr(ev, attr, tmp, sizeof(tmp)))
 						append_str(buf, &pos, bufsz, tmp);
 					p = ae + 1;
 				} else {
@@ -908,6 +934,8 @@ static int parent_cb(const char *syspath, void *data)
 
 	if (pattern_match(k->value, subject, k->pat_type)) {
 		ctx->found = 1;
+		if (k->op == OP_MATCH_EQ)
+			snprintf(matched_parent, sizeof(matched_parent), "%s", syspath);
 		return 0;	/* stop walking */
 	}
 	return 1;		/* keep walking */
@@ -1101,6 +1129,8 @@ static int rule_matches(const struct rule *r, struct uevent *ev)
 {
 	int i;
 
+	matched_parent[0] = 0;
+
 	for (i = 0; i < r->nkeys; i++) {
 		const struct rule_key *k = &r->keys[i];
 
@@ -1627,6 +1657,7 @@ int rules_apply(struct rule_list *list, struct uevent *ev)
 		      uevent_action_str(ev->action), ev->devpath ?: "");
 
 		exec_rule(r, ev);
+		matched_parent[0] = 0;
 
 		/* GOTO takes effect after this rule's assignments are applied */
 		goto_label = NULL;

keventd/uevent.c

@@ -507,6 +507,62 @@ static struct devrule *find_rule(struct uevent *ev)
 	return &devrules[NELEMS(devrules) - 1];
 }
 
+/*
+ * Build the relative target of a symlink at /dev/<link> that points at the
+ * device node /dev/<node>, e.g.
+ *
+ *   link "input/by-id/usb-X-event-kbd", node "input/event17" -> "../event17"
+ *   link "disk/by-id/ata-X",            node "sda"           -> "../../sda"
+ *
+ * Both arguments are relative to /dev.  The directory components the link
+ * and the node have in common are dropped, then one "../" is emitted for
+ * each directory the link still has to climb out of.
+ */
+static int symlink_relative_target(const char *link, const char *node,
+				   char *buf, size_t len)
+{
+	const char *l = link, *n = node;
+	size_t off = 0;
+	int ret;
+
+	while (*l == '/')
+		l++;
+	while (*n == '/')
+		n++;
+
+	/* Skip leading directories shared by both paths */
+	for (;;) {
+		const char *ls = strchr(l, '/');
+		const char *ns = strchr(n, '/');
+
+		if (!ls || !ns || ls - l != ns - n || strncmp(l, n, ls - l))
+			break;
+		l = ls + 1;
+		n = ns + 1;
+		while (*l == '/')
+			l++;
+		while (*n == '/')
+			n++;
+	}
+
+	buf[0] = 0;
+	for (; (l = strchr(l, '/')); l++) {
+		while (l[1] == '/')
+			l++;
+		if (!l[1])
+			break;
+		ret = snprintf(buf + off, len - off, "../");
+		if (ret < 0 || (size_t)ret >= len - off)
+			return -1;
+		off += ret;
+	}
+
+	ret = snprintf(buf + off, len - off, "%s", n);
+	if (ret < 0 || (size_t)ret >= len - off)
+		return -1;
+	return 0;
+}
+
 /*
  * Create device node in /dev.
  */
@@ -903,9 +959,6 @@ int symlink_add(struct uevent *ev)
 	for (i = 0; i < ev->applied.nsymlinks; i++) {
 		const char *devname, *sl;
 		char link[PATH_MAX], target[PATH_MAX];
-		size_t toff = 0;
-		int depth = 0;
-		const char *p;
 
 		sl = ev->applied.symlinks[i];
 		if (!sl || !*sl)
@@ -915,19 +968,15 @@ int symlink_add(struct uevent *ev)
 		if (!devname)
 			continue;
 
-		/* basename of devname for the symlink target */
-		const char *base = strrchr(devname, '/');
-		base = base ? base + 1 : devname;
-
-		/* Count '/' in symlink path to determine relative depth */
-		for (p = sl; *p; p++)
-			if (*p == '/')
-				depth++;
-
-		target[0] = 0;
-		for (int d = 0; d < depth; d++)
-			toff += snprintf(target + toff, sizeof(target) - toff, "../");
-		snprintf(target + toff, sizeof(target) - toff, "%s", base);
+		/*
+		 * The node lives at /dev/<devname>, which is not necessarily
+		 * directly in /dev (input/event17, dri/card0, ...), so the
+		 * target must be computed relative to the link's directory.
+		 */
+		if (symlink_relative_target(sl, devname, target, sizeof(target))) {
+			logit(LOG_WARNING, "Symlink %s -> %s: path too long", sl, devname);
+			continue;
+		}
 
 		snprintf(link, sizeof(link), "/dev/%s", sl);
 		symlink_create(target, link, ev->devpath ?: "");