Coverity fixes

doc/ChangeLog.md

@@ -121,6 +121,10 @@ All notable changes to the project are documented in this file.
 - WebUI: a WiFi interface can be switched between station, access point
   and mesh point from the interface editor. The mode used to be fixed
   when the interface was created
+- OSPF sometimes learned no routes over a link after a routing change
+  was applied while the link went down and up: the adjacency came up
+  but the interface was stuck at network type Null and left out of the
+  router's LSA. Seen about once in thirty runs on virtual machines
 
 [relsup]: https://github.com/kernelkit/infix/blob/main/doc/releases.md
 [snmp]: https://www.kernelkit.org/infix/latest/snmp/

patches/frr/10.5.5/0004-lib-realize-a-config-created-interface-on-its-first-.patch

@@ -0,0 +1,62 @@
+From 13d16871ee1da466d42ab2bb72de3516de2ebb5d Mon Sep 17 00:00:00 2001
+From: Joachim Wiberg <troglobit@gmail.com>
+Date: Tue, 29 Sep 2026 20:46:52 +0200
+Subject: [PATCH 4/4] lib: realize a config-created interface on its first
+ state update
+Organization: Wires
+
+A daemon that names an interface in its config, e.g. ospfd with an
+"interface e7" stanza, creates it as a placeholder before connecting
+to zebra.  Zebra sends INTERFACE_UP/DOWN and ADDRESS_ADD to every
+client as they happen, also to one that has not yet received the
+interface dump it asked for.  zebra_interface_state_read() finds the
+placeholder by name and gives it the ifindex, but only
+zebra_interface_add_read() calls if_new_via_zapi(), so the if_real
+hook does not run.  An ADDRESS_ADD then finds the interface by index
+and the daemon acts on an interface it was never told is real.
+
+For ospfd this leaves the ospf_interface with network type 0, "Null"
+in show ip ospf interface: the type is filled in by the if_real hook
+and only copied to the ospf_interface when it is created.  Its
+router-LSA then lacks the link, so no routes are learned across it
+although the adjacency reaches Full.  Seen when a link flap coincides
+with an ospfd restart, FRRouting/frr#4178.
+
+A state update carries the same data as an add, so treat the first one
+for a placeholder as the add.  A delete for a placeholder then reads as
+add and delete, which is what the dump would have given.
+
+Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
+---
+ lib/zclient.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
+
+diff --git a/lib/zclient.c b/lib/zclient.c
+index 48fb42872a..9a444dc041 100644
+--- a/lib/zclient.c
++++ b/lib/zclient.c
+@@ -2851,6 +2851,7 @@ struct interface *zebra_interface_state_read(struct stream *s, vrf_id_t vrf_id)
+ {
+ 	struct interface *ifp;
+ 	char ifname_tmp[IFNAMSIZ + 1] = {};
++	bool unreal;
+
+ 	/* Read interface name. */
+ 	STREAM_GET(ifname_tmp, s, IFNAMSIZ);
+@@ -2864,8 +2865,13 @@ struct interface *zebra_interface_state_read(struct stream *s, vrf_id_t vrf_id)
+ 		return NULL;
+ 	}
+
++	unreal = ifp->ifindex == IFINDEX_INTERNAL;
+ 	zebra_interface_if_set_value(s, ifp);
+
++	/* Created from config ahead of INTERFACE_ADD, see FRRouting/frr#4178 */
++	if (unreal && ifp->ifindex != IFINDEX_INTERNAL)
++		if_new_via_zapi(ifp);
++
+ 	return ifp;
+ stream_failure:
+ 	return NULL;
+-- 
+2.43.0
+

src/bin/files.c

@@ -105,6 +105,8 @@ static int complete(const char *word)
 	strlcpy(dir, word, sizeof(dir));
 
 	slash = strrchr(dir, '/');
+	if (!slash)
+		return 0;
 	base = word + (slash - dir) + 1;
 	slash[1] = 0;
 

src/bin/rename.c

@@ -2,13 +2,13 @@
 #include "config.h"
 
 #include <errno.h>
+#include <fcntl.h>
 #include <getopt.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <limits.h>
 #include <sys/stat.h>
-#include <unistd.h>
 
 #include "util.h"
 
@@ -43,13 +43,12 @@ static int mkparent(const char *path)
 static int do_rename(const char *from, const char *to)
 {
 	char *src = NULL, *dst = NULL;
+	int rc = 1, err;
 	mode_t mode;
-	int rc = 1;
 
 	src = cfg_adjust(from, NULL, sanitize);
-	if (!src || access(src, F_OK)) {
-		fprintf(stderr, "%s: %s: no such file, or not an allowed path\n",
-			prognm, from);
+	if (!src) {
+		fprintf(stderr, "%s: %s: not an allowed path\n", prognm, from);
 		goto out;
 	}
 
@@ -59,17 +58,23 @@ static int do_rename(const char *from, const char *to)
 		goto out;
 	}
 
-	if (!force && !access(dst, F_OK) && !yorn("Overwrite existing file %s", dst))
-		goto out;
-
 	if (mkparent(dst)) {
 		fprintf(stderr, "%s: failed creating directory for %s: %s\n",
 			prognm, dst, strerror(errno));
 		goto out;
 	}
 
-	if (rename(src, dst)) {
-		if (errno == EXDEV)
+	err = renameat2(AT_FDCWD, src, AT_FDCWD, dst, force ? 0 : RENAME_NOREPLACE);
+	if (err && errno == EEXIST) {
+		if (!yorn("Overwrite existing file %s", dst))
+			goto out;
+		err = rename(src, dst);
+	}
+
+	if (err) {
+		if (errno == ENOENT)
+			fprintf(stderr, "%s: %s: no such file\n", prognm, from);
+		else if (errno == EXDEV)
 			fprintf(stderr, "%s: %s and %s are on different file systems,"
 				" use copy and remove\n", prognm, src, dst);
 		else

src/confd/src/containers.c

@@ -80,7 +80,7 @@ static int add(const char *name, struct lyd_node *cif)
 	FILE *fp, *ap;
 
 	snprintf(script, sizeof(script), "%s.sh", name);
-	fp = fopenf("w", "%s/%s", _PATH_CONT, script);
+	fp = fopenfp(0700, NULL, "%s/%s", _PATH_CONT, script);
 	if (!fp) {
 		ERRNO("Failed creating container script %s/%s", _PATH_CONT, script);
 		return SR_ERR_SYS;
@@ -319,7 +319,6 @@ static int add(const char *name, struct lyd_node *cif)
 		fprintf(fp, " %s", string);
 
 	fprintf(fp, "\n");
-	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
 	if (lydx_is_enabled(cif, "enabled")) {

src/confd/src/factory-default.c

@@ -18,11 +18,6 @@ static int factory_reset(sr_session_ctx_t *session, uint32_t sub_id, const char
 
 int factory_default_rpc_init(struct confd *confd)
 {
-	int rc;
-
-	REGISTER_RPC(confd->session, "/ietf-factory-default:factory-reset", factory_reset, NULL, &confd->fsub);
-	return SR_ERR_OK;
-fail:
-	ERROR("failed: %s", sr_strerror(rc));
-	return rc;
+	return register_rpc(confd->session, "/ietf-factory-default:factory-reset",
+			    factory_reset, NULL, &confd->fsub);
 }

src/confd/src/factory.c

@@ -35,10 +35,6 @@ static int rpc(sr_session_ctx_t *session, uint32_t sub_id, const char *xpath,
 
 int factory_rpc_init(struct confd *confd)
 {
-	int rc;
-	REGISTER_RPC(confd->session, "/infix-factory-default:factory-default", rpc, NULL, &confd->fsub);
-	return SR_ERR_OK;
-fail:
-	ERROR("failed: %s", sr_strerror(rc));
-	return rc;
+	return register_rpc(confd->session, "/infix-factory-default:factory-default",
+			    rpc, NULL, &confd->fsub);
 }

src/confd/src/ip.c

@@ -19,8 +19,8 @@ int ifchange_cand_infer_ipv4_prefix(sr_session_ctx_t *session, const sr_val_t *v
 	sr_error_t err = SR_ERR_OK;
 	struct in_addr ina;
 	uint32_t addr;
-	char *xpath;
 	size_t cnt;
+	int len;
 
 	if (!strstr(val->xpath, ":ipv4/address[") || fnmatch("*]/ip", val->xpath, 0))
 		return SR_ERR_OK;
@@ -37,16 +37,12 @@ int ifchange_cand_infer_ipv4_prefix(sr_session_ctx_t *session, const sr_val_t *v
 	else
 		return SR_ERR_OK;	/* class D/E, no default */
 
-	xpath = strdup(val->xpath);
-	if (!xpath)
-		return SR_ERR_SYS;
-	*strrchr(xpath, '/') = 0;
-
-	err = srx_nitems(session, &cnt, "%s/prefix-length", xpath);
+	/* Strip the /ip matched above to get the address list entry */
+	len = strlen(val->xpath) - strlen("/ip");
+	err = srx_nitems(session, &cnt, "%.*s/prefix-length", len, val->xpath);
 	if (!err && !cnt)
-		err = srx_set_item(session, &inferred, 0, "%s/prefix-length", xpath);
+		err = srx_set_item(session, &inferred, 0, "%.*s/prefix-length", len, val->xpath);
 
-	free(xpath);
 	return err;
 }
 

src/confd/src/main.c

@@ -421,8 +421,8 @@ static int maybe_migrate(const char *path)
 
 	NOTE("%s config version %s vs confd %s, migrating ...", path, file_ver, CONFD_VERSION);
 
-	mkpath(backup_dir, 0770);
-	chown(backup_dir, 0, 10); /* root:wheel */
+	if (mkpath(backup_dir, 0770) || chown(backup_dir, 0, 10)) /* root:wheel */
+		WARN("Cannot create %s: %m", backup_dir);
 
 	snprintf(backup, sizeof(backup), "%s/%s", backup_dir, basenm(path));
 	rc = systemf("migrate -i -b \"%s\" \"%s\"", backup, path);

src/confd/src/support.c

@@ -71,9 +71,10 @@ static void strip_lf(unsigned char *str)
 /* Same mode as the tool gives it, our umask is stricter */
 static const char *workdir(void)
 {
-	if (!mkdir(SUPPORT_DIR, 0755))
-		chmod(SUPPORT_DIR, 0755);
-	else if (errno != EEXIST)
+	if (!mkdir(SUPPORT_DIR, 0755)) {
+		if (chmod(SUPPORT_DIR, 0755))
+			WARN("Cannot set mode of %s: %s", SUPPORT_DIR, strerror(errno));
+	} else if (errno != EEXIST)
 		return SUPPORT_TMP;
 
 	if (access(SUPPORT_DIR, W_OK))
@@ -375,10 +376,6 @@ static int rpc_collect(sr_session_ctx_t *session, uint32_t sub_id, const char *p
 
 int support_rpc_init(struct confd *confd)
 {
-	int rc = 0;
-
-	REGISTER_RPC(confd->session, "/infix-system:support-collect",
-		     rpc_collect, NULL, &confd->sub);
-fail:
-	return rc;
+	return register_rpc(confd->session, "/infix-system:support-collect",
+			    rpc_collect, NULL, &confd->sub);
 }

src/klish-plugin-infix/src/infix.c

@@ -666,7 +666,8 @@ static void ensure_known_hosts(const struct passwd *pw)
 	if (fd < 0)
 		return; /* Already exists, or unrecoverable error */
 
-	fchown(fd, pw->pw_uid, pw->pw_gid);
+	if (fchown(fd, pw->pw_uid, pw->pw_gid))
+		unlink(path);
 	close(fd);
 }
 

src/statd/iface.c

@@ -99,22 +99,23 @@ static void iface_parse(struct iface_ctx *ctx, struct nlmsghdr *nlh, int dump)
 	struct ifinfomsg *ifi = NLMSG_DATA(nlh);
 	int len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
 	uint8_t operstate = IF_OPER_UNKNOWN;
-	const char *name = NULL;
+	char name[IFNAMSIZ] = "";
 	struct rtattr *rta;
 	struct iface *l;
 
 	for (rta = IFLA_RTA(ifi); RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) {
 		switch (rta->rta_type & NLA_TYPE_MASK) {
 		case IFLA_IFNAME:
-			name = RTA_DATA(rta);
+			snprintf(name, sizeof(name), "%.*s", (int)RTA_PAYLOAD(rta),
+				 (char *)RTA_DATA(rta));
 			break;
 		case IFLA_OPERSTATE:
 			operstate = *(uint8_t *)RTA_DATA(rta);
 			break;
 		}
 	}
 
-	if (!name)
+	if (!name[0])
 		return;
 
 	if (nlh->nlmsg_type == RTM_DELLINK) {

test/case/interfaces/lag_failure/test.adoc

@@ -21,6 +21,7 @@ image::topology.svg[LACP Aggregate w/ Degraded Link topology, align=center, scal
 
 . Set up topology and attach to target DUTs
 . Set up link aggregate, lag0, between dut1 and dut2
+. Wait for LACP to synchronize both links
 . Initial connectivity check ...
 . Verify failure modes
 

test/case/interfaces/lag_failure/test.py

@@ -15,7 +15,7 @@
 import infamy
 import infamy.lag
 from infamy.netns import TPMR
-from infamy.util import parallel
+from infamy.util import parallel, until
 
 IPH = "192.168.2.1"
 IP1 = "192.168.2.41"
@@ -135,6 +135,10 @@ def dut_init(dut, addr, peer):
             parallel(lambda: dut_init(dut1, IP1, IP2),
                      lambda: dut_init(dut2, IP2, IP1))
 
+        with test.step("Wait for LACP to synchronize both links"):
+            until(lambda: all(infamy.lag.lacp_synced(dut, dut["link1"], dut["link2"])
+                              for dut in (dut1, dut2)), attempts=60)
+
         with test.step("Initial connectivity check ..."):
             ns.must_reach(IP2, timeout=30)
 

test/case/misc/support_collect/test.adoc

@@ -1,6 +1,6 @@
 === Support Data Collection
 
-ifdef::topdoc[:imagesdir: {topdoc}../../misc/support_collect]
+ifdef::topdoc[:imagesdir: {topdoc}../../test/case/misc/support_collect]
 
 ==== Description
 

test/infamy/lag.py

@@ -1,4 +1,4 @@
-from . import topology
+from . import iface, topology
 
 def edge_mappings(les, pes):
     """Specialized topology edge mapper for LAG tests
@@ -24,3 +24,18 @@ def links_compatible(candidate):
     for candidate in topology.edge_mappings(les, pes):
         if links_compatible(candidate):
             yield candidate
+
+
+def lacp_synced(target, *ports):
+    """True when all ports are collecting and distributing, as seen by both ends"""
+    for port in ports:
+        data = target.get_data(iface.get_xpath(port)) or {}
+        for entry in data.get("interfaces", {}).get("interface", []):
+            # netconf presents lag-port, restconf prefixes it with the model
+            lagport = entry.get("lag-port") or entry.get("infix-interfaces:lag-port") or {}
+            lacp = lagport.get("lacp", {})
+            for state in (lacp.get("actor-state", []), lacp.get("partner-state", [])):
+                if not {"collecting", "distributing"} <= set(state):
+                    return False
+
+    return True

test/infix-test-skill.md

@@ -15,3 +15,26 @@ Info about the Infix regression tests.
   and `provides` attributes, and prints the mapping in the log. A test is
   skipped when no mapping fits.
 - Logs: `test/.log/<log id>/output/`. Full guide: `doc/testing.md`.
+
+## Debugging a local QEMU run
+
+- `make test-sh` keeps the DUTs and the `infamy0` container running after
+  a failure, `make test` tears them down. The DUTs are QEMU guests with
+  384 MB RAM, one host tap per port (`d2a` is port a of dut2, and so on).
+- The log maps the test's logical names to DUTs (`R1: dut2`) and prints
+  the mgmt address it connected to, e.g. `fe80::2a0:85ff:fe00:201%d2a`.
+- Run commands on a DUT over SSH from inside the container, admin/admin:
+
+      podman exec infamy0 sshpass -p admin ssh -o StrictHostKeyChecking=no \
+          -o UserKnownHostsFile=/dev/null admin@fe80::2a0:85ff:fe00:201%d2a \
+          'vtysh -c "show ip ospf neighbor"'
+
+  `admin` can `sudo -n` and is in `frrvty`, so `vtysh`, `/var/log/messages`,
+  `dmesg`, `initctl status` and `sysrepocfg -X -d operational -x <xpath>`
+  are all reachable this way. `test/console dut2` attaches to the serial
+  console instead. The system is Finit and sysklogd, there is no journal.
+- Run a subset: list the tests in a yaml under `test/case/` with `case:`
+  paths relative to that directory, then
+  `make test TESTS=$PWD/test/case/subset.yaml`. Repeat an entry under
+  different names to loop a flaky test. `INFAMY_ARGS=--transport=netconf`
+  (or `restconf`) forces the transport, otherwise it is picked per run.