Merge pull request #2442 from pqarmitage/updates

Allow for Ethernet padding when checking receive packet size
acassen · Jul 9, 2024 · ccb4d9c · ccb4d9c
2 parents 1c4a7df + c9da511
commit ccb4d9c
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 14 deletions.
diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c
@@ -40,6 +40,7 @@
 #include <netinet/ip6.h>
 #include <stdint.h>
 #include <net/if_arp.h>
+#include <linux/if_ether.h>
 #include <net/ethernet.h>
 #ifdef _NETWORK_TIMESTAMP_
 #include <linux/net_tstamp.h>
@@ -90,6 +91,11 @@
 #include "ipvswrapper.h"
 #endif
 
+/* Ideally we would use a struct from a system header to determine the
+ * size of a vlan tag, but there doesn't seem to be one exposed to
+ * user space. */
+#define VLAN_TAG_SIZE	4
+
 /* If we don't have certain configuration, then we can optimise the
  * resources that keepalived uses. These are cleared by start_vrrp()
  * in clear_summary_flags() and set in vrrp_complete_instance()
@@ -841,12 +847,27 @@ vrrp_check_packet(vrrp_t *vrrp, const vrrphdr_t *hd, const char *buffer, ssize_t
 	 * packet (including fixed fields, and IPvX address(es)).
 	 */
 	if (buflen != expected_len) {
-		log_message(LOG_INFO, "(%s) vrrp packet too %s, length %zu and expect %zu",
-			      vrrp->iname,
-			      buflen > expected_len ? "long" : "short",
-			      buflen, expected_len);
-		++vrrp->stats->packet_len_err;
-		return VRRP_PACKET_KO;
+		/* Allow for Ethernet frame padding. If there is padding, the
+		 * frame length (excluding FCS) is 60 octets (ETH_ZLEN).
+		 * The Ethernet header (14 bytes - ETH_HLEN) and any Vlan
+		 * headers (4 bytes each) are removed before we receive the
+		 * packet.
+		 * Padding added is ETH_ZLEN - ETH_HLEN - expected_len, or
+		 * multiples of 4 (Vlan header) less than that. Checking the
+		 * amount of padding added can therefore only be done modulo 4.
+		 */
+		if (expected_len < ETH_ZLEN - ETH_HLEN &&
+		    expected_len < buflen &&
+		    (buflen - expected_len) % VLAN_TAG_SIZE == (VLAN_TAG_SIZE - (ETH_ZLEN - ETH_HLEN) % VLAN_TAG_SIZE) % VLAN_TAG_SIZE) {
+			/* This is OK, there is some padding */
+		} else {
+			log_message(LOG_INFO, "(%s) vrrp packet too %s, length %zu and expect %zu",
+				      vrrp->iname,
+				      buflen > expected_len ? "long" : "short",
+				      buflen, expected_len);
+			++vrrp->stats->packet_len_err;
+			return VRRP_PACKET_KO;
+		}
 	}
 
 	/* MUST verify that the IPv4 TTL/IPv6 HL is 255 (but not if unicast) */
@@ -974,11 +995,19 @@ vrrp_check_packet(vrrp_t *vrrp, const vrrphdr_t *hd, const char *buffer, ssize_t
 
 	/* Check the IP header total packet length matches what we received */
 	if (vrrp->family == AF_INET && ntohs(ip->tot_len) != buflen) {
-		log_message(LOG_INFO,
-		       "(%s) ip_tot_len mismatch against received length. %d and received %zu",
-		       vrrp->iname, ntohs(ip->tot_len), buflen);
-		++vrrp->stats->packet_len_err;
-		return VRRP_PACKET_KO;
+		/* Allow for Ethernet frame padding. See earlier comment
+		 * for details. */
+		if (buflen <= ETH_ZLEN - ETH_HLEN &&
+		    ntohs(ip->tot_len) < buflen &&
+		    (buflen - ntohs(ip->tot_len)) % VLAN_TAG_SIZE == (VLAN_TAG_SIZE - (ETH_ZLEN - ETH_HLEN) % VLAN_TAG_SIZE) % VLAN_TAG_SIZE) {
+			/* This is OK, there is some padding */
+		} else {
+			log_message(LOG_INFO,
+			       "(%s) ip_tot_len mismatch against received length. %d and received %zu",
+			       vrrp->iname, ntohs(ip->tot_len), buflen);
+			++vrrp->stats->packet_len_err;
+			return VRRP_PACKET_KO;
+		}
 	}
 
 	/* MUST verify the VRRP checksum. Kernel takes care of checksum mismatch incase of IPv6. */

diff --git a/keepalived/vrrp/vrrp_data.c b/keepalived/vrrp/vrrp_data.c
@@ -611,7 +611,7 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
 			conf_write(fp, "   Master router = %s", inet_sockaddrtos(&vrrp->master_saddr));
 			conf_write(fp, "   Master priority = %d", vrrp->master_priority);
 			if (vrrp->version == VRRP_VERSION_3)
-				conf_write(fp, "   Master advert int = %.2f sec", vrrp->master_adver_int / TIMER_HZ_DOUBLE);
+				conf_write(fp, "   Master advert interval = %u milli-sec", vrrp->master_adver_int / (TIMER_HZ / 1000));
 		}
 	}
 	if (vrrp->flags) {
@@ -747,8 +747,6 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
 		(vrrp->adver_int / (TIMER_HZ / 1000)),
 		(vrrp->version == VRRP_VERSION_2) ? "sec" : "milli-sec");
 	conf_write(fp, "   Last advert sent = %ld.%6.6ld", vrrp->last_advert_sent.tv_sec, vrrp->last_advert_sent.tv_usec);
-	if (vrrp->state == VRRP_STATE_BACK && vrrp->version == VRRP_VERSION_3)
-		conf_write(fp, "   Master advert interval = %u milli-sec", vrrp->master_adver_int / (TIMER_HZ / 1000));
 #ifdef _WITH_FIREWALL_
 	conf_write(fp, "   Accept = %s", vrrp->accept ? "enabled" : "disabled");
 #endif