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);