From 46d5e750d754a888a5aeec5c6d3613e3d317a88b Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 16:50:33 +0100 Subject: [PATCH 1/6] confd: Simplify GRE and VXLAN iface setup CID 507415 from coverity was the origin for this change. As it turns out, the unreachable MAC address generation for VXLANs was hidden by the generic setup (netdag_gen_link_addr()) saving the day. That is likely something we want to phase out though, so the VXLAN code should stand on its own. Align the GRE code while we're here, because why not? :) --- src/confd/src/infix-if-gre.c | 7 ++++--- src/confd/src/infix-if-vxlan.c | 27 ++++++++++++--------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/confd/src/infix-if-gre.c b/src/confd/src/infix-if-gre.c index 4db1a6767..5f17e6f3f 100644 --- a/src/confd/src/infix-if-gre.c +++ b/src/confd/src/infix-if-gre.c @@ -5,17 +5,18 @@ int gre_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip) { - const char *ifname, *local, *remote; + const char *local, *remote; struct lyd_node *gre; int ipv6; - ifname = lydx_get_cattr(cif, "name"); gre = lydx_get_child(cif, "gre"); local = lydx_get_cattr(gre, "local"); remote = lydx_get_cattr(gre, "remote"); + ipv6 = !!strstr(local, ":"); - fprintf(ip, "link add name %s", ifname); + fprintf(ip, "link add name %s", + lydx_get_cattr(cif, "name")); switch (iftype_from_iface(cif)) { case IFT_GRE: diff --git a/src/confd/src/infix-if-vxlan.c b/src/confd/src/infix-if-vxlan.c index dda711588..e270c8c47 100644 --- a/src/confd/src/infix-if-vxlan.c +++ b/src/confd/src/infix-if-vxlan.c @@ -5,24 +5,21 @@ int vxlan_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip) { - const char *ifname, *local, *remote, *mac = NULL; - const char *vni, *remote_port; - struct lyd_node *node = NULL; + struct lyd_node *vxlan = NULL; - ifname = lydx_get_cattr(cif, "name"); - node = lydx_get_descendant(lyd_child(cif), "vxlan", NULL); - if (!node) + vxlan = lydx_get_descendant(lyd_child(cif), "vxlan", NULL); + if (!vxlan) return -EINVAL; - local = lydx_get_cattr(node, "local"); - remote = lydx_get_cattr(node, "remote"); - vni = lydx_get_cattr(node, "vni"); - remote_port = lydx_get_cattr(node, "remote-port"); - fprintf(ip, "link add name %s type vxlan id %s local %s remote %s dstport %s", ifname, vni, local, remote, remote_port); - if (mac) - fprintf(ip, "address %s\n", mac); - else - fprintf(ip, "\n"); + fprintf(ip, "link add name %s type vxlan id %s local %s remote %s dstport %s", + lydx_get_cattr(cif, "name"), + lydx_get_cattr(vxlan, "vni"), + lydx_get_cattr(vxlan, "local"), + lydx_get_cattr(vxlan, "remote"), + lydx_get_cattr(vxlan, "remote-port")); + link_gen_address(cif, ip); + + fputc('\n', ip); return 0; } From 7e252ba9417df5812d08cee59d2b903cb8522ec8 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 17:00:59 +0100 Subject: [PATCH 2/6] confd: Fix unchecked return codes Fix coverity CIDS: - 464651 - 464652 - 464655 - 507413 - 507414 --- src/confd/src/infix-dhcp-server.c | 3 ++- src/confd/src/infix-if-bridge-mcd.c | 4 +++- src/confd/src/infix-services.c | 15 ++++++++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/confd/src/infix-dhcp-server.c b/src/confd/src/infix-dhcp-server.c index 23f7b285a..a942ddf11 100644 --- a/src/confd/src/infix-dhcp-server.c +++ b/src/confd/src/infix-dhcp-server.c @@ -289,7 +289,8 @@ static void del(const char *subnet, struct lyd_node *cfg) fclose(next); /* Replace old leases file */ - rename(DNSMASQ_LEASES"+", DNSMASQ_LEASES); + if (rename(DNSMASQ_LEASES"+", DNSMASQ_LEASES)) + ERRNO("Failed switching to new %s", DNSMASQ_LEASES); } static int change(sr_session_ctx_t *session, uint32_t sub_id, const char *module, diff --git a/src/confd/src/infix-if-bridge-mcd.c b/src/confd/src/infix-if-bridge-mcd.c index c54e4573e..8c3f0bd6a 100644 --- a/src/confd/src/infix-if-bridge-mcd.c +++ b/src/confd/src/infix-if-bridge-mcd.c @@ -136,6 +136,8 @@ int bridge_mcd_gen(struct lyd_node *cifs) } out_remove: - remove("/etc/mc.d/bridges.conf.next"); + if (remove("/etc/mc.d/bridges.conf.next")) + ERRNO("Failed removing /etc/mc.d/bridges.conf.next"); + return err; } diff --git a/src/confd/src/infix-services.c b/src/confd/src/infix-services.c index 13046bb1b..001eba202 100644 --- a/src/confd/src/infix-services.c +++ b/src/confd/src/infix-services.c @@ -349,7 +349,8 @@ static int lldp_change(sr_session_ctx_t *session, uint32_t sub_id, const char *m if (erase(LLDP_CONFIG)) ERRNO("Failed to remove old %s", LLDP_CONFIG); - rename(LLDP_CONFIG_NEXT, LLDP_CONFIG); + if (rename(LLDP_CONFIG_NEXT, LLDP_CONFIG)) + ERRNO("Failed switching to new %s", LLDP_CONFIG); } else if (erase(LLDP_CONFIG)) @@ -531,7 +532,10 @@ static int change_keystore_cb(sr_session_ctx_t *session, uint32_t sub_id, const if(rmrf(SSH_HOSTKEYS)) { ERRNO("Failed to remove old SSH hostkeys: %d", errno); } - rename(SSH_HOSTKEYS_NEXT, SSH_HOSTKEYS); + + if (rename(SSH_HOSTKEYS_NEXT, SSH_HOSTKEYS)) + ERRNO("Failed switching to new %s", SSH_HOSTKEYS); + svc_change(session, event, "/infix-services:ssh", "ssh", "sshd"); } return SR_ERR_OK; @@ -564,7 +568,12 @@ static int change_keystore_cb(sr_session_ctx_t *session, uint32_t sub_id, const } private_key = lydx_get_cattr(change, "cleartext-private-key"); public_key = lydx_get_cattr(change, "public-key"); - mkdir(SSH_HOSTKEYS_NEXT, 0600); + + if (mkdir(SSH_HOSTKEYS_NEXT, 0600) && (errno != EEXIST)) { + ERRNO("Failed creating %s", SSH_HOSTKEYS_NEXT); + rc = SR_ERR_INTERNAL; + } + if(systemf("/usr/libexec/infix/mksshkey %s %s %s %s", name, SSH_HOSTKEYS_NEXT, public_key, private_key)) rc = SR_ERR_INTERNAL; } From f6879e24ccc9412136ae0acc5ddb22e7f6734406 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 17:13:28 +0100 Subject: [PATCH 3/6] bin: Fix unchecked return codes Fix coverity CID 425560 --- src/bin/copy.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/bin/copy.c b/src/bin/copy.c index a157106a0..c16368d6d 100644 --- a/src/bin/copy.c +++ b/src/bin/copy.c @@ -84,13 +84,10 @@ static void set_owner(const char *fn, const char *user) struct passwd *pw; pw = getpwnam(user); - if (!pw) { - fprintf(stderr, ERRMSG "setting owner %s on %s: %s\n", fn, user, strerror(errno)); + if (pw && !chmod(fn, 0660) && !chown(fn, pw->pw_uid, pw->pw_gid)) return; - } - chmod(fn, 0660); - chown(fn, pw->pw_uid, pw->pw_gid); + fprintf(stderr, ERRMSG "setting owner %s on %s: %s\n", fn, user, strerror(errno)); } static const char *infix_ds(const char *text, struct infix_ds **ds) From fe741a92f380830eabe4ee69d37e92fe2a1f265b Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 17:17:30 +0100 Subject: [PATCH 4/6] confd: Explicitly return OK when registration is successful Fix coverity CID 464654 --- src/confd/src/ietf-keystore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/confd/src/ietf-keystore.c b/src/confd/src/ietf-keystore.c index f078526b7..2e1e38f78 100644 --- a/src/confd/src/ietf-keystore.c +++ b/src/confd/src/ietf-keystore.c @@ -148,6 +148,7 @@ int ietf_keystore_init(struct confd *confd) REGISTER_CHANGE(confd->session, "ietf-keystore", "/ietf-keystore:keystore//.", SR_SUBSCR_UPDATE, change_cb, confd, &confd->sub); + return SR_ERR_OK; fail: if(rc) ERROR("%s failed: %s", __func__, sr_strerror(rc)); From 3c0679991df009fe0a9b8d80995ea233c796f5c8 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 17:21:33 +0100 Subject: [PATCH 5/6] confd: bridge: Refactor init_snippets() to avoid unreachable code Fix coverity CID 464653 --- src/confd/src/infix-if-bridge.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/confd/src/infix-if-bridge.c b/src/confd/src/infix-if-bridge.c index 3f8d32780..55ffaf852 100644 --- a/src/confd/src/infix-if-bridge.c +++ b/src/confd/src/infix-if-bridge.c @@ -426,10 +426,8 @@ static int init_snippets(struct ixif_br *br, struct lyd_node *dif, struct lyd_no br->cif = cif; br->ip = ip; - err = snippet_open(&br->bropts); - if (err) - goto err; + err = snippet_open(&br->bropts); err = err ? : snippet_open(&br->init.vlan); err = err ? : snippet_open(&br->init.mcast); err = err ? : snippet_open(&br->init.mdb); @@ -443,15 +441,11 @@ static int init_snippets(struct ixif_br *br, struct lyd_node *dif, struct lyd_no snippet_close(&br->init.mdb, NULL); snippet_close(&br->init.mcast, NULL); snippet_close(&br->init.vlan, NULL); - goto err_close_bropts; + snippet_close(&br->bropts, NULL); + return err; } return 0; - -err_close_bropts: - snippet_close(&br->bropts, NULL); -err: - return err; } static int collect_snippets(struct ixif_br *br) From 4d7dde536657c49fd89c9c0253e9bf324a029f69 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Sat, 15 Feb 2025 23:12:53 +0100 Subject: [PATCH 6/6] test: bridge_stp_basic: Skip on multi-chip mv88e6xxx systems Until we fix kernelkit/linux#2, tag the affected nodes as providing "broken-stp", and skip this test on those rigs. --- test/case/ietf_interfaces/bridge_stp_basic/test.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/case/ietf_interfaces/bridge_stp_basic/test.py b/test/case/ietf_interfaces/bridge_stp_basic/test.py index 6f9fbf78d..ef01ba3eb 100755 --- a/test/case/ietf_interfaces/bridge_stp_basic/test.py +++ b/test/case/ietf_interfaces/bridge_stp_basic/test.py @@ -16,6 +16,13 @@ import infamy from infamy.util import parallel, until + +def stp_compatible(physical, logical): + # Multi-chip mv88e6xxx systems do not flush their ATU's correctly + # (tracked by kernelkit/linux#2) + return "broken-stp" not in physical["provides"] + + def addbr(dut): ip = { "A": "10.0.0.101", @@ -133,7 +140,7 @@ def port_role(dut, port): with infamy.Test() as test: with test.step("Set up topology and attach to target DUT"): - env = infamy.Env() + env = infamy.Env(nodes_compatible=stp_compatible) a, b, c, d = parallel(lambda: env.attach("A"), lambda: env.attach("B"), lambda: env.attach("C"), lambda: env.attach("D"))