git: 91489043435f - main - ipfw: Fix broken length checks on routing messages

From: John Baldwin <jhb_at_FreeBSD.org>
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);