Coverity fixes - #1665
Merged
Merged
Coverity fixes#1665
Conversation
Coverity Scan flags a time-of-check to time-of-use race (CID 564389) in rename: both files are checked with access(), and then renamed. The destination can appear after the check and be overwritten without asking, and the source can go away before the rename. Let the rename itself do the checking. RENAME_NOREPLACE fails with EEXIST if the destination exists, so we ask before overwriting, and a missing source is reported from ENOENT. Also guard the strrchr() in the path completion of files. It cannot return NULL for an absolute path, but Coverity cannot see that (CID 564393). Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Coverity Scan reports two defects in the support-collect RPC: - CID 564390: the return value of chmod() on /var/lib/support is not checked, so a failure to set the mode goes unnoticed - CID 564391: identical branches, the REGISTER_RPC macro jumps to a fail label that is the next statement anyway Log a warning if chmod() fails, and return register_rpc() directly. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
The name in IFLA_IFNAME was used as a string straight from the receive buffer, relying on the kernel to NUL terminate it. Coverity Scan reports it as an unterminated string (CID 564392). Copy it into an IFNAMSIZ buffer, bounded by the attribute payload. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Both factory RPC init functions logged a failure to subscribe twice, once in register_rpc() and again after the jump to their fail label. Return register_rpc() directly, like the support-collect RPC. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
A container script that could not be made executable went unnoticed
until the container failed to set up:
setup: /run/containers/NAME.sh does not exist or is not executable.
Open the script with fopenfp(), which sets the mode and logs failure.
Also warn if the backup directory for a configuration migration cannot
be created or given to root:wheel.
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
The ssh command creates ~/.ssh/known_hosts as root, then hands it over to the user. If fchown() failed, the file stayed owned by root, ssh could never add a host key to it, and it was never created again. Remove it on failure, so the next ssh command can try again. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
troglobit
marked this pull request as ready for review
September 28, 2026 04:45
Contributor
|
The normal LACP test failed, but also 0091 OSPF Default Route Advertise, can you please try to triage it @troglobit |
Coverity Scan reports the strrchr() that strips the ip leaf from a copy of the address xpath as a possible NULL dereference (CID 564405). It cannot be, the fnmatch() above only lets xpaths ending in /ip through. Drop the copy and print the parent xpath with a length limit instead. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
The failure sequence started about a second after the aggregate was
created, while LACP was still negotiating the second link. In QEMU
that takes about nine seconds, so the first failover measured the
rest of the negotiation: 7 to 9 s in a normal run, and past the 30 s
ping budget in two CI runs on 2026-09-28:
# block | forward | FAIL after 30.01s
The second link never carried traffic before that either, which is
why blocking it always passed instantly.
Wait for both member links to be collecting and distributing on both
DUTs before the first connectivity check. Failover then completes in
about a second, and blocking the second link is a real failover.
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Found with "make test-spec" Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Contributor
Author
|
Triaged. 0049 was a race in the test: it started breaking links ~1 s after creating the LAG, while LACP was still negotiating the second member (~9 s in QEMU). The same signature hit 0091 is one failure in 26 runs with a NETCONF session reset mid-step; couldn't reproduce. |
mattiaswal
approved these changes
Sep 29, 2026
The test skill said where the logs are, not how to get at a DUT that make test-sh leaves running, so debugging a failure meant finding the container, the mgmt address and the ssh incantation again every time. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
About one run in thirty of the OSPF tests in QEMU, one router had a full adjacency and a complete link state database but no routes from its neighbor, and the neighbor none from it. Its router-LSA lacked the link, and show ip ospf interface said "Network Type Null" for it. ospfd names the interface in its config, so it exists as a placeholder before ospfd connects to zebra. Zebra sends interface state and address events to every client as they happen, also to one that has not yet received the interface dump. A state event finds the placeholder by name and gives it the ifindex without running the hook that sets the default network type, the address event that follows creates the OSPF interface with type 0, and the dump arrives too late. A link flap during an ospfd restart does it, which is what applying a routing configuration involves. FRRouting/frr#4178. Treat the first state event for a placeholder like the interface add it stands in for. With the dump held back to keep the window open, 19 of 20 restarts hit it before, none after. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Coverity Scan found five new issues on its w39-26 run. This PR addresses all and a few other, similar findings, along the way.
All new findings are for functionality added or modified in this release cycle.
Checklist
Tick relevant boxes, this PR is-a or has-a: