From bac2019ffc778b9e194d2d59f56db5838eef5a75 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 10 Oct 2026 09:54:40 +0200 Subject: [PATCH 1/3] confd: refuse over-long interface names in pppmon The interface name from the command line was copied into a fixed size netlink request without a length check, so a name longer than the 256-byte attribute buffer overflowed the stack. Coverity Scan reports it as a string of unknown size passed to syslog (CID 564681). Reject names the kernel would not accept anyway, before using them. Signed-off-by: Joachim Wiberg --- src/confd/bin/pppmon.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/confd/bin/pppmon.c b/src/confd/bin/pppmon.c index f1477de07..77708673d 100644 --- a/src/confd/bin/pppmon.c +++ b/src/confd/bin/pppmon.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -182,6 +183,12 @@ int main(int argc, char *argv[]) ifname = argv[optind]; openlog("pppmon", LOG_PID | LOG_PERROR, LOG_DAEMON); + /* Same limit as the kernel, also bounds the netlink request */ + if (strlen(ifname) >= IFNAMSIZ) { + syslog(LOG_ERR, "interface name too long, max %d chars", IFNAMSIZ - 1); + return 1; + } + fd = open("/dev/ppp", O_RDWR); if (fd < 0) { syslog(LOG_ERR, "%s: failed opening /dev/ppp: %m", ifname); From 2bfd50a9729e38faafaf17f3e5526cb9ac415751 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 10 Oct 2026 09:54:43 +0200 Subject: [PATCH 2/3] confd: check rename of generated PPP files If the new peers file could not be put in place, pppd was restarted on the old one and the change reported as applied. Coverity Scan reports the unchecked rename() and remove() (CID 564682, 564679). Fail the change if rename() fails. Failing to remove an unchanged scratch file is harmless, the next change overwrites it. Signed-off-by: Joachim Wiberg --- src/confd/src/if-ppp.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/confd/src/if-ppp.c b/src/confd/src/if-ppp.c index dc345184b..975f773a7 100644 --- a/src/confd/src/if-ppp.c +++ b/src/confd/src/if-ppp.c @@ -156,22 +156,25 @@ static int ppp_gen_settings(struct lyd_node *cif) return 0; } -/* Replace file with file+ if they differ, returns 1 if it did */ +/* Replace file with file+ if they differ: 1 if replaced, 0 if not, -1 on error */ static int ppp_update(const char *fmt, const char *ifname) { char path[256], next[260]; - int changed; snprintf(path, sizeof(path), fmt, ifname); snprintf(next, sizeof(next), "%s+", path); - changed = systemf("cmp -s %s %s", path, next) != 0; - if (changed) - rename(next, path); - else - remove(next); + if (!systemf("cmp -s %s %s", path, next)) { + (void)remove(next); + return 0; + } + + if (rename(next, path)) { + ERRNO("%s: failed replacing %s", ifname, path); + return -1; + } - return changed; + return 1; } /* @@ -224,8 +227,11 @@ int ppp_gen(sr_session_ctx_t *session, struct lyd_node *dif, struct lyd_node *ci return err; /* Finit restarts pppd on its own when the env file changes */ - ppp_update(PPP_SETTINGS, ifname); + if (ppp_update(PPP_SETTINGS, ifname) < 0) + return -EIO; restart = ppp_update(PPP_PEERS, ifname); + if (restart < 0) + return -EIO; if (lydx_is_enabled(cif, "enabled")) { finit_enablef("pppd@%s", ifname); From 0092250c242cd8803a6fae13fba50053a4231eac Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 10 Oct 2026 09:54:43 +0200 Subject: [PATCH 3/3] confd: fail locate if the iitod socket timeouts cannot be set The timeouts keep a stuck iitod from blocking confd, so the call must not go ahead without them. Coverity Scan reports the unchecked setsockopt() calls (CID 564680). Signed-off-by: Joachim Wiberg --- src/confd/src/locate.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/confd/src/locate.c b/src/confd/src/locate.c index d28ccdde0..ee2de581d 100644 --- a/src/confd/src/locate.c +++ b/src/confd/src/locate.c @@ -44,8 +44,11 @@ static json_t *iitod_call(json_t *req, const char **err) } /* Do not let a stuck iitod block confd, this also bounds connect() */ - setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); - setsockopt(sd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); + if (setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) || + setsockopt(sd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv))) { + *err = "failed setting socket timeout"; + goto done; + } if (connect(sd, (struct sockaddr *)&sun, sizeof(sun))) { if (errno == ENOENT)