confd: refactor hostnamefmt()

This refactor, and massive code simplification, is a follow-up to the double free discovery in 3b4d2b3. Beacuse a hostname in Linux can never be more than 64 chars, we can safely use a stack buffer instead of calloc(), reducing complexity and simplifying the code further with the use of strlcat(). Signed-off-by: Joachim Wiberg <[email protected]>
kernelkit · Oct 16, 2024 · 99e0a2e · 99e0a2e
1 parent 0d0e2b1
commit 99e0a2e
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 66 deletions.
diff --git a/src/confd/src/core.h b/src/confd/src/core.h
@@ -172,7 +172,8 @@ int ietf_interfaces_init(struct confd *confd);
 int ietf_syslog_init(struct confd *confd);
 
 /* ietf-system.c */
-int ietf_system_init(struct confd *confd);
+int ietf_system_init (struct confd *confd);
+int hostnamefmt      (struct confd *confd, const char *fmt, char *buf, size_t len);
 
 /* infix-containers.c */
 #ifdef CONTAINERS

diff --git a/src/confd/src/ietf-system.c b/src/confd/src/ietf-system.c
@@ -1595,23 +1595,6 @@ static char *get_mac(struct confd *confd, char *mac, size_t len)
 	return mac;
 }
 
-#define REPLACE_SPECIFIER(replacement)				\
-	do {							\
-		size_t repl_len = strlen(replacement);		\
-		char *ptr;					\
-								\
-		hostlen += repl_len;				\
-		ptr = realloc(hostname, hostlen + 1);		\
-		if (!ptr) {					\
-			free(hostname);				\
-			free(*fmt);				\
-			return -1;				\
-		}						\
-		hostname = ptr;					\
-		strlcat(hostname, replacement, hostlen + 1);	\
-		j += repl_len;					\
-	} while (0)
-
 /*
  * Decode hostname format specifiers (non-standard, Infix specifc)
  *
@@ -1622,82 +1605,62 @@ static char *get_mac(struct confd *confd, char *mac, size_t len)
  *
  * NOTE: to be forward compatible, any unknown % combination will be silently consumed.
  *       E.g., "example-%z" will become "example-"
- *
- * XXX: PLEASE REFACTOR THIS INTO A PYTHON HELPER FOR FUTURE EXTENSIONS, OR BUGS!
  */
-int hostnamefmt(struct confd *confd, char **fmt)
+int hostnamefmt(struct confd *confd, const char *fmt, char *buf, size_t len)
 {
-	size_t hostlen, fmt_len;
-	char *hostname;
 	char mac[10];
-	size_t i, j;
-
-	if (!fmt || !*fmt || !strchr(*fmt, '%'))
-		return 0;
+	size_t i;
 
-	hostlen = fmt_len = strlen(*fmt);
-	hostname = calloc(hostlen + 1, sizeof(char));
-	if (!hostname)
+	if (!fmt || !*fmt)
 		return -1;
 
-	for (i = 0, j = 0; i < fmt_len; i++) {
-		if ((*fmt)[i] == '%') {
-			switch ((*fmt)[++i]) {
+	memset(buf, 0, len);
+
+	for (i = 0; i < strlen(fmt); i++) {
+		if (fmt[i] == '%') {
+			switch (fmt[++i]) {
 			case 'i':
-				REPLACE_SPECIFIER(id);
+				strlcat(buf, id, len);
 				break;
 			case 'h':
-				REPLACE_SPECIFIER(nm);
+				strlcat(buf, nm, len);
 				break;
 			case 'm':
-				REPLACE_SPECIFIER(get_mac(confd, mac, sizeof(mac)));
+				strlcat(buf, get_mac(confd, mac, sizeof(mac)), len);
 				break;
 			case '%':
-				if (j < hostlen) {
-					hostname[j++] = '%';
-					hostname[j]   = 0;
-				}
+				strlcat(buf, "%", len);
 				break;
 			default:
 				break; /* Unknown, skip */
 			}
 		} else {
-			if (j < hostlen) {
-				hostname[j++] = (*fmt)[i];
-				hostname[j]   = 0;
-			}
+			char ch[2] = { fmt[i], 0 };
+			strlcat(buf, ch, len);
 		}
 	}
 
-	free(*fmt);
-	*fmt = hostname;
-
 	return 0;
 }
 
-#undef REPLACE_SPECIFIER
-
 static int change_hostname(sr_session_ctx_t *session, uint32_t sub_id, const char *module,
 	const char *xpath, sr_event_t event, unsigned request_id, void *priv)
 {
 	struct confd *confd = (struct confd *)priv;
 	const char *hostip = "127.0.1.1";
-	char *hostnm, buf[256];
+	char  hostnm[65], buf[256];
+	const char *fmt;
 	FILE *nfp, *fp;
 	int err, fd;
 
 	if (event != SR_EV_DONE)
 		return SR_ERR_OK;
 
-	hostnm = srx_get_str(session, "%s", xpath);
-	if (!hostnm) {
+	fmt = srx_get_str(session, "%s", xpath);
+	if (!fmt) {
 	fallback:
-		hostnm = strdup(nm);
-		if (!hostnm) {
-			err = SR_ERR_NO_MEMORY;
-			goto err;
-		}
-	} else if (hostnamefmt(confd, &hostnm))
+		strlcpy(hostnm, nm, sizeof(hostnm));
+	} else if (hostnamefmt(confd, fmt, hostnm, sizeof(hostnm)))
 		goto fallback;
 
 	err = sethostname(hostnm, strlen(hostnm));
@@ -1754,9 +1717,6 @@ static int change_hostname(sr_session_ctx_t *session, uint32_t sub_id, const cha
 	systemf("avahi-set-host-name %s", hostnm);
 	systemf("initctl -nbq touch netbrowse");
 err:
-	if (hostnm)
-		free(hostnm);
-
 	if (err) {
 		ERROR("Failed activating changes.");
 		return err;

diff --git a/src/confd/src/infix-containers.c b/src/confd/src/infix-containers.c
@@ -21,8 +21,6 @@
 #define  ACTIVE_QUEUE "/var/lib/containers/active"
 #define  LOGGER       "logger -t container -p local1.notice"
 
-int hostnamefmt(struct confd *confd, char **fmt);
-
 
 static int add(const char *name, struct lyd_node *cif)
 {
@@ -50,12 +48,12 @@ static int add(const char *name, struct lyd_node *cif)
 		fprintf(fp, " --dns-search %s", lyd_get_value(node));
 
 	if ((string = lydx_get_cattr(cif, "hostname"))) {
-		char *fmt = (char *)string;
+		char buf[65];
 
-		if (hostnamefmt(&confd, &fmt))
+		if (hostnamefmt(&confd, string, buf, sizeof(buf)))
 			ERRNO("%s: failed setting custom hostname", name);
 		else
-			fprintf(fp, " --hostname %s", fmt);
+			fprintf(fp, " --hostname %s", buf);
 	}
 
 	if (lydx_is_enabled(cif, "read-only"))