From 5acbaa86ac6df1407b0877a82ab1276f6e3ef557 Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Sat, 26 Apr 2025 13:09:19 +0100 Subject: [PATCH 1/4] Use inet_pton() in gen_mcode6(). The function needs to translate a [well-formed] IPv6 address string to a struct in6_addr. Although pcap_nametoaddrinfo() in fact does that, it overshoots the problem space unnecessarily and requires managing of the associated freeaddrinfo() call. Also the "resolved to multiple address" condition is never true because in this case the string is an address, not a hostname, and cannot translate to more than one binary address. Switch to inet_pton(), which fits the problem space much better, and verify that it has not failed (very unlikely, but still). --- gencode.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/gencode.c b/gencode.c index 0c2f6e0d9b..d4ffe2f329 100644 --- a/gencode.c +++ b/gencode.c @@ -7112,10 +7112,8 @@ struct block * gen_mcode6(compiler_state_t *cstate, const char *s, bpf_u_int32 masklen, struct qual q) { - struct addrinfo *res; - struct in6_addr *addr; + struct in6_addr addr; struct in6_addr mask; - struct block *b; bpf_u_int32 a[4], m[4]; /* Same as in gen_hostop6(). */ /* @@ -7125,13 +7123,12 @@ gen_mcode6(compiler_state_t *cstate, const char *s, bpf_u_int32 masklen, if (setjmp(cstate->top_ctx)) return (NULL); - res = pcap_nametoaddrinfo(s); - if (!res) - bpf_error(cstate, "invalid ip6 address %s", s); - cstate->ai = res; - if (res->ai_next) - bpf_error(cstate, "%s resolved to multiple address", s); - addr = &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr; + /* + * If everything works correctly, this call never fails: a string that + * is valid for HID6 in the lexer is valid for inet_pton(). + */ + if (1 != inet_pton(AF_INET6, s, &addr)) + bpf_error(cstate, "'%s' is not a valid IPv6 address", s); if (masklen > sizeof(mask.s6_addr) * 8) bpf_error(cstate, "mask length must be <= %zu", sizeof(mask.s6_addr) * 8); @@ -7142,7 +7139,7 @@ gen_mcode6(compiler_state_t *cstate, const char *s, bpf_u_int32 masklen, (0xff << (8 - masklen % 8)) & 0xff; } - memcpy(a, addr, sizeof(a)); + memcpy(a, &addr, sizeof(a)); memcpy(m, &mask, sizeof(m)); if ((a[0] & ~m[0]) || (a[1] & ~m[1]) || (a[2] & ~m[2]) || (a[3] & ~m[3])) { @@ -7158,10 +7155,7 @@ gen_mcode6(compiler_state_t *cstate, const char *s, bpf_u_int32 masklen, /* FALLTHROUGH */ case Q_NET: - b = gen_host6(cstate, addr, &mask, q.proto, q.dir, q.addr); - cstate->ai = NULL; - freeaddrinfo(res); - return b; + return gen_host6(cstate, &addr, &mask, q.proto, q.dir, q.addr); default: // Q_GATEWAY only (see the grammar) From c137f6d78caf4d81c4ec9cee785a1f1e80c33115 Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Sat, 26 Apr 2025 13:25:50 +0100 Subject: [PATCH 2/4] Rename a variable in dqkw() for consistency. --- gencode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gencode.c b/gencode.c index d4ffe2f329..a32a69d193 100644 --- a/gencode.c +++ b/gencode.c @@ -935,7 +935,7 @@ pqkw(const unsigned id) static const char * dqkw(const unsigned id) { - const char * map[] = { + const char * tokens[] = { [Q_SRC] = "src", [Q_DST] = "dst", [Q_OR] = "src or dst", @@ -947,7 +947,7 @@ dqkw(const unsigned id) [Q_RA] = "ra", [Q_TA] = "ta", }; - return qual2kw("dir", id, map, sizeof(map) / sizeof(map[0])); + return qual2kw("dir", id, tokens, sizeof(tokens) / sizeof(tokens[0])); } // ATM keywords From b927be2b11460c7060c2ceff47b5ea1b95b3a474 Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Mon, 28 Apr 2025 22:46:24 +0100 Subject: [PATCH 3/4] WIP: Simplify gen_gateway(). In gen_scode() case Q_GATEWAY delegate the remaining memory management and error handling away; now if gen_gateway() returns, it always returns a non-NULL value. In gen_gateway() handle all errors as early as possible; instead of testing whether ai_addr is NULL test whether ai_family is set to the AF of interest, when that's the case, ai_addr is not NULL (this is how gen_scode() implements case Q_HOST). --- gencode.c | 113 ++++++++++++++++++++++++------------------------------ 1 file changed, 51 insertions(+), 62 deletions(-) diff --git a/gencode.c b/gencode.c index a32a69d193..6cadcb2feb 100644 --- a/gencode.c +++ b/gencode.c @@ -680,8 +680,7 @@ static struct block *gen_host(compiler_state_t *, bpf_u_int32, bpf_u_int32, int, int, int); static struct block *gen_host6(compiler_state_t *, struct in6_addr *, struct in6_addr *, int, int, int); -static struct block *gen_gateway(compiler_state_t *, const char *, - struct addrinfo *, int); +static struct block *gen_gateway(compiler_state_t *, const char *, const int); static struct block *gen_ip_proto(compiler_state_t *, const uint8_t); static struct block *gen_ip6_proto(compiler_state_t *, const uint8_t); static struct block *gen_ipfrag(compiler_state_t *); @@ -1054,6 +1053,7 @@ assert_maxval(compiler_state_t *cstate, const char *name, #define ERRSTR_802_11_ONLY_KW "'%s' is valid for 802.11 syntax only" #define ERRSTR_INVALID_QUAL "'%s' is not a valid qualifier for '%s'" #define ERRSTR_UNKNOWN_MAC48HOST "unknown Ethernet-like host '%s'" +#define ERRSTR_UNKNOWN_HOST "unknown host '%s'" // Validate a port/portrange proto qualifier and map to an IP protocol number. static int @@ -5312,68 +5312,66 @@ gen_mac48host_byname(compiler_state_t *cstate, const char *name, * to qualify it with a direction. */ static struct block * -gen_gateway(compiler_state_t *cstate, const char *name, - struct addrinfo *alist, int proto) +gen_gateway(compiler_state_t *cstate, const char *name, const int proto) { - struct block *b0, *b1, *tmp; - struct addrinfo *ai; - struct sockaddr_in *sin; - switch (proto) { case Q_DEFAULT: case Q_IP: case Q_ARP: case Q_RARP: - b0 = gen_mac48host_byname(cstate, name, Q_OR, "gateway"); - b1 = NULL; - for (ai = alist; ai != NULL; ai = ai->ai_next) { + break; + default: + bpf_error(cstate, ERRSTR_INVALID_QUAL, pqkw(proto), "gateway"); + } + + struct block *b0 = gen_mac48host_byname(cstate, name, Q_OR, "gateway"); + + cstate->ai = pcap_nametoaddrinfo(name); + if (cstate->ai == NULL) + bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); + struct block *b1 = NULL; + for (struct addrinfo *ai = cstate->ai; ai != NULL; ai = ai->ai_next) { + // Pick IPv4 addresses only. + if (ai->ai_family != AF_INET) + continue; + /* + * Generate an entry for it. + */ + struct sockaddr_in *sin = + (struct sockaddr_in *)ai->ai_addr; + struct block *tmp = gen_host(cstate, + ntohl(sin->sin_addr.s_addr), + 0xffffffff, proto, Q_OR, Q_HOST); + /* + * Is it the *first* IPv4 address? + */ + if (b1 == NULL) { /* - * Does it have an address? + * Yes, so start with it. */ - if (ai->ai_addr != NULL) { - /* - * Yes. Is it an IPv4 address? - */ - if (ai->ai_addr->sa_family == AF_INET) { - /* - * Generate an entry for it. - */ - sin = (struct sockaddr_in *)ai->ai_addr; - tmp = gen_host(cstate, - ntohl(sin->sin_addr.s_addr), - 0xffffffff, proto, Q_OR, Q_HOST); - /* - * Is it the *first* IPv4 address? - */ - if (b1 == NULL) { - /* - * Yes, so start with it. - */ - b1 = tmp; - } else { - /* - * No, so OR it into the - * existing set of - * addresses. - */ - gen_or(b1, tmp); - b1 = tmp; - } - } - } - } - if (b1 == NULL) { + b1 = tmp; + } else { /* - * No IPv4 addresses found. + * No, so OR it into the + * existing set of + * addresses. */ - return (NULL); + gen_or(b1, tmp); + b1 = tmp; } - gen_not(b1); - gen_and(b0, b1); - return b1; } - bpf_error(cstate, ERRSTR_INVALID_QUAL, pqkw(proto), "gateway"); - /*NOTREACHED*/ + freeaddrinfo(cstate->ai); + cstate->ai = NULL; + + if (b1 == NULL) { + /* + * No IPv4 addresses found. + */ + bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); + } + gen_not(b1); + gen_and(b0, b1); + return b1; } static struct block * @@ -6889,16 +6887,7 @@ gen_scode(compiler_state_t *cstate, const char *name, struct qual q) return b; case Q_GATEWAY: - res = pcap_nametoaddrinfo(name); - cstate->ai = res; - if (res == NULL) - bpf_error(cstate, "unknown host '%s'", name); - b = gen_gateway(cstate, name, res, proto); - cstate->ai = NULL; - freeaddrinfo(res); - if (b == NULL) - bpf_error(cstate, "unknown host '%s'", name); - return b; + return gen_gateway(cstate, name, proto); case Q_PROTO: real_proto = lookup_proto(cstate, name, proto); From 3ae4822f66866d389e82a982b69cd5857ab2c60e Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Thu, 22 May 2025 16:28:47 +0100 Subject: [PATCH 4/4] WIP: Factor gen_host46_byname() out. [skip ci] --- gencode.c | 158 ++++++++++++++++++---------------------------- testprogs/TESTrun | 2 + 2 files changed, 65 insertions(+), 95 deletions(-) diff --git a/gencode.c b/gencode.c index 6cadcb2feb..89c606da1f 100644 --- a/gencode.c +++ b/gencode.c @@ -680,6 +680,8 @@ static struct block *gen_host(compiler_state_t *, bpf_u_int32, bpf_u_int32, int, int, int); static struct block *gen_host6(compiler_state_t *, struct in6_addr *, struct in6_addr *, int, int, int); +static struct block *gen_host46_byname(compiler_state_t *, const char *, + const u_char, const u_char, const u_char); static struct block *gen_gateway(compiler_state_t *, const char *, const int); static struct block *gen_ip_proto(compiler_state_t *, const uint8_t); static struct block *gen_ip6_proto(compiler_state_t *, const uint8_t); @@ -5192,6 +5194,55 @@ gen_host6(compiler_state_t *cstate, struct in6_addr *addr, /*NOTREACHED*/ } +static struct block * +gen_host46_byname(compiler_state_t *cstate, const char *name, + const u_char proto4, const u_char proto6, const u_char dir) +{ + if (! (cstate->ai = pcap_nametoaddrinfo(name))) + bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); + struct block *ret = NULL; + struct in6_addr mask128; + memset(&mask128, 0xff, sizeof(mask128)); + for (struct addrinfo *ai = cstate->ai; ai; ai = ai->ai_next) { + struct block *tmp = NULL; + switch (ai->ai_family) { + case AF_INET: + switch (proto4) { + case Q_IP: + case Q_ARP: + case Q_RARP: + case Q_DEFAULT: + struct sockaddr_in *sin4 = + (struct sockaddr_in *)ai->ai_addr; + tmp = gen_host(cstate, ntohl(sin4->sin_addr.s_addr), + 0xffffffff, proto4, dir, Q_HOST); + } + break; + case AF_INET6: + switch (proto6) { + case Q_IPV6: + case Q_DEFAULT: + struct sockaddr_in6 *sin6 = + (struct sockaddr_in6 *)ai->ai_addr; + tmp = gen_host6(cstate, &sin6->sin6_addr, + &mask128, proto6, dir, Q_HOST); + } + break; + } + if (! tmp) + continue; + if (ret) + gen_or(ret, tmp); + ret = tmp; + } + free(cstate->ai); + cstate->ai = NULL; + + if (! ret) + bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); + return ret; +} + static unsigned char is_mac48_linktype(const int linktype) { @@ -5325,50 +5376,7 @@ gen_gateway(compiler_state_t *cstate, const char *name, const int proto) } struct block *b0 = gen_mac48host_byname(cstate, name, Q_OR, "gateway"); - - cstate->ai = pcap_nametoaddrinfo(name); - if (cstate->ai == NULL) - bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); - struct block *b1 = NULL; - for (struct addrinfo *ai = cstate->ai; ai != NULL; ai = ai->ai_next) { - // Pick IPv4 addresses only. - if (ai->ai_family != AF_INET) - continue; - /* - * Generate an entry for it. - */ - struct sockaddr_in *sin = - (struct sockaddr_in *)ai->ai_addr; - struct block *tmp = gen_host(cstate, - ntohl(sin->sin_addr.s_addr), - 0xffffffff, proto, Q_OR, Q_HOST); - /* - * Is it the *first* IPv4 address? - */ - if (b1 == NULL) { - /* - * Yes, so start with it. - */ - b1 = tmp; - } else { - /* - * No, so OR it into the - * existing set of - * addresses. - */ - gen_or(b1, tmp); - b1 = tmp; - } - } - freeaddrinfo(cstate->ai); - cstate->ai = NULL; - - if (b1 == NULL) { - /* - * No IPv4 addresses found. - */ - bpf_error(cstate, ERRSTR_UNKNOWN_HOST, name); - } + struct block *b1 = gen_host46_byname(cstate, name, proto, proto, Q_OR); gen_not(b1); gen_and(b0, b1); return b1; @@ -6707,14 +6715,8 @@ gen_scode(compiler_state_t *cstate, const char *name, struct qual q) { int proto = q.proto; int dir = q.dir; - int tproto; bpf_u_int32 mask, addr; - struct addrinfo *res, *res0; - struct sockaddr_in *sin4; - int tproto6; - struct sockaddr_in6 *sin6; - struct in6_addr mask128; - struct block *b, *tmp; + struct block *b; int port, real_proto; bpf_u_int32 port1, port2; @@ -6751,55 +6753,21 @@ gen_scode(compiler_state_t *cstate, const char *name, struct qual q) */ bpf_error(cstate, "invalid DECnet address '%s'", name); } else { - memset(&mask128, 0xff, sizeof(mask128)); - res0 = res = pcap_nametoaddrinfo(name); - if (res == NULL) - bpf_error(cstate, "unknown host '%s'", name); - cstate->ai = res; - b = tmp = NULL; - tproto = proto; - tproto6 = proto; + u_char tproto = q.proto; + u_char tproto6 = q.proto; if (cstate->off_linktype.constant_part == OFFSET_NOT_SET && tproto == Q_DEFAULT) { + /* + * For certain DLTs have "host NAME" mean + * "ip host NAME or ip6 host NAME", but not + * "arp host NAME or rarp host NAME" (here may + * be not the best place for this though). + */ tproto = Q_IP; tproto6 = Q_IPV6; } - for (res = res0; res; res = res->ai_next) { - switch (res->ai_family) { - case AF_INET: - if (tproto == Q_IPV6) - continue; - - sin4 = (struct sockaddr_in *) - res->ai_addr; - tmp = gen_host(cstate, ntohl(sin4->sin_addr.s_addr), - 0xffffffff, tproto, dir, q.addr); - break; - case AF_INET6: - if (tproto6 == Q_IP) - continue; - - sin6 = (struct sockaddr_in6 *) - res->ai_addr; - tmp = gen_host6(cstate, &sin6->sin6_addr, - &mask128, tproto6, dir, q.addr); - break; - default: - continue; - } - if (b) - gen_or(b, tmp); - b = tmp; - } - cstate->ai = NULL; - freeaddrinfo(res0); - if (b == NULL) { - bpf_error(cstate, "unknown host '%s'%s", name, - (proto == Q_DEFAULT) - ? "" - : " for specified address family"); - } - return b; + return gen_host46_byname(cstate, name, tproto, + tproto6, q.dir); } case Q_PORT: diff --git a/testprogs/TESTrun b/testprogs/TESTrun index ca149cb7dd..3b19eee429 100755 --- a/testprogs/TESTrun +++ b/testprogs/TESTrun @@ -8776,6 +8776,8 @@ my @accept_blocks = ( 'ip src or dst noeth-ipv4-ipv6.host123.libpcap.test', 'ip src or dst host noeth-ipv4-noipv6.host123.libpcap.test', 'ip src or dst host noeth-ipv4-ipv6.host123.libpcap.test', + # This does not mean "arp host" or "rarp host" because for + # DLT_RAW off_linktype.constant_part == OFFSET_NOT_SET. 'host noeth-ipv4-noipv6.host123.libpcap.test', 'src or dst noeth-ipv4-noipv6.host123.libpcap.test', 'src or dst host noeth-ipv4-noipv6.host123.libpcap.test',