git: 91489043435f - main - ipfw: Fix broken length checks on routing messages
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 22 Apr 2025 02:07:49 UTC
The branch main has been updated by jhb:
URL: https://cgit.FreeBSD.org/src/commit/?id=91489043435f1f98a03d1cd5138a6ce37408e92f
commit 91489043435f1f98a03d1cd5138a6ce37408e92f
Author: John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2025-04-21 20:53:15 +0000
Commit: John Baldwin <jhb@FreeBSD.org>
CommitDate: 2025-04-22 02:00:14 +0000
ipfw: Fix broken length checks on routing messages
Subtracting unsigned and signed types of the same rank yields an
unsigned value that is never less than 0. Rewrite the checks to use
the pattern of 'if (msglen < <desired size>)' instead of
'if (msglen - <desired_size> < 0)' to avoid the subtraction.
To avoid adding lots of casts to appease -Wsign-compare, use a
separate ssize_t variable for the return value of read(2) and convert
msglen to size_t.
While here, fix the first check against the size of the route message
header which was inverted and would have rejected all valid messages
if not for the unsigned vs signed bug causing all of the checks to be
broken.
sbin/ipfw/ipfw2.c: In function 'ipfw_rtsock_monitor':
sbin/ipfw/ipfw2.c:6088:43: error: comparison of unsigned expression in '< 0' is always false [-Werror=type-limits]
6088 | if (sizeof(*hdr) - msglen < 0)
| ^
Reported by: GCC -Wtype-limits
Fixes: 3c76623ad553 ("ipfw: add 'internal monitor' subcommand to capture rtsock messages.")
---
sbin/ipfw/ipfw2.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/sbin/ipfw/ipfw2.c b/sbin/ipfw/ipfw2.c
index a95e7b0318da..2addc0295f0f 100644
--- a/sbin/ipfw/ipfw2.c
+++ b/sbin/ipfw/ipfw2.c
@@ -6072,7 +6072,8 @@ ipfw_rtsock_monitor(const char *filter)
struct sockaddr *sa;
struct sockaddr_dl *sdl;
ipfwlog_rtsock_hdr_v2 *loghdr;
- ssize_t msglen;
+ ssize_t nread;
+ size_t msglen;
int rtsock;
rtsock = socket(PF_ROUTE, SOCK_RAW, AF_IPFWLOG);
@@ -6080,12 +6081,13 @@ ipfw_rtsock_monitor(const char *filter)
err(EX_UNAVAILABLE, "socket(AF_IPFWLOG)");
bp_alloc(&bp, 4096);
for (;;) {
- msglen = read(rtsock, msg, sizeof(msg));
- if (msglen < 0) {
+ nread = read(rtsock, msg, sizeof(msg));
+ if (nread < 0) {
warn("read()");
continue;
}
- if (sizeof(*hdr) - msglen < 0)
+ msglen = nread;
+ if (msglen < sizeof(*hdr))
continue;
hdr = (struct rt_msghdr *)msg;
@@ -6098,7 +6100,7 @@ ipfw_rtsock_monitor(const char *filter)
msglen -= sizeof(*hdr);
sdl = (struct sockaddr_dl *)(hdr + 1);
- if (msglen - sizeof(*sdl) < 0 || msglen - SA_SIZE(sdl) < 0 ||
+ if (msglen < sizeof(*sdl) || msglen < SA_SIZE(sdl) ||
sdl->sdl_family != AF_IPFWLOG ||
sdl->sdl_type != 2 /* version */ ||
sdl->sdl_alen != sizeof(*loghdr))
@@ -6112,7 +6114,7 @@ ipfw_rtsock_monitor(const char *filter)
continue;
sa = (struct sockaddr *)((char *)sdl + SA_SIZE(sdl));
- if (msglen - SA_SIZE(sa) < 0)
+ if (msglen < SA_SIZE(sa))
continue;
msglen -= SA_SIZE(sa);
@@ -6131,7 +6133,7 @@ ipfw_rtsock_monitor(const char *filter)
bprint_sa(&bp, sa);
sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa));
- if (msglen - SA_SIZE(sa) < 0)
+ if (msglen < SA_SIZE(sa))
continue;
msglen -= SA_SIZE(sa);
@@ -6146,7 +6148,7 @@ ipfw_rtsock_monitor(const char *filter)
sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa));
if ((hdr->rtm_addrs & (1 << RTAX_GENMASK)) != 0 &&
- msglen - SA_SIZE(sa) >= 0) {
+ msglen >= SA_SIZE(sa)) {
msglen -= SA_SIZE(sa);
bprintf(&bp, ", nh ");
bprint_sa(&bp, sa);