ndp: fix icmp6 option parsing

Expecting that packet data is contiguous is wrong. Plus the current code is not safe against (forged?) truncated packets at the NDP protocol level. Rework existing helper and avoid pointer arithmetics. Fixes: fb72a4d ("ip6: implement dynamic node address discovery") Signed-off-by: David Marchand <[email protected]>
DPDK · Dec 23, 2024 · 6a43d4c · 6a43d4c
1 parent 7135627
commit 6a43d4c
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 15 deletions.
diff --git a/modules/infra/datapath/gr_icmp6.h b/modules/infra/datapath/gr_icmp6.h
@@ -132,9 +132,11 @@ struct icmp6_opt_lladdr {
 	struct rte_ether_addr mac;
 } __rte_aligned(2) __rte_packed;
 
-static inline bool
-icmp6_get_opt(const struct icmp6_opt *opt, size_t ip6_len, uint8_t type, void *value) {
-	while (ip6_len >= 8 && opt != NULL) {
+static inline bool icmp6_get_opt(struct rte_mbuf *mbuf, size_t offset, uint8_t type, void *value) {
+	const struct icmp6_opt *opt;
+	struct icmp6_opt popt;
+
+	while ((opt = rte_pktmbuf_read(mbuf, offset, sizeof(*opt), &popt)) != NULL) {
 		if (opt->type != type)
 			goto next;
 
@@ -148,8 +150,7 @@ icmp6_get_opt(const struct icmp6_opt *opt, size_t ip6_len, uint8_t type, void *v
 			break;
 		}
 next:
-		ip6_len -= opt->len * 8;
-		opt = RTE_PTR_ADD(opt, opt->len * 8);
+		offset += opt->len * 8;
 	}
 	return false;
 }

diff --git a/modules/ip6/control/nexthop.c b/modules/ip6/control/nexthop.c
@@ -128,20 +128,14 @@ void ndp_probe_input_cb(struct rte_mbuf *m) {
 		// replaced in ndp_ns_input_process to avoid copying the whole IPv6 header.
 		target = ns->target;
 		lladdr_found = icmp6_get_opt(
-			PAYLOAD(ns),
-			rte_pktmbuf_pkt_len(m) - sizeof(*ns),
-			ICMP6_OPT_SRC_LLADDR,
-			&mac
+			m, sizeof(*icmp6) + sizeof(*ns), ICMP6_OPT_SRC_LLADDR, &mac
 		);
 		break;
 	case ICMP6_TYPE_NEIGH_ADVERT:
 		na = PAYLOAD(icmp6);
 		target = na->target;
 		lladdr_found = icmp6_get_opt(
-			PAYLOAD(na),
-			rte_pktmbuf_pkt_len(m) - sizeof(*ns),
-			ICMP6_OPT_TARGET_LLADDR,
-			&mac
+			m, sizeof(*icmp6) + sizeof(*na), ICMP6_OPT_TARGET_LLADDR, &mac
 		);
 		break;
 	default:

diff --git a/modules/ip6/datapath/ndp_na_input.c b/modules/ip6/datapath/ndp_na_input.c
@@ -84,7 +84,7 @@ static uint16_t ndp_na_input_process(
 		ASSERT_NDP(remote != NULL);
 
 		lladdr_found = icmp6_get_opt(
-			PAYLOAD(na), rte_pktmbuf_pkt_len(mbuf), ICMP6_OPT_TARGET_LLADDR, &lladdr
+			mbuf, sizeof(*icmp6) + sizeof(*na), ICMP6_OPT_TARGET_LLADDR, &lladdr
 		);
 		// If the link layer has addresses and no Target Link-Layer Address
 		// option is included, the receiving node SHOULD silently discard the

diff --git a/modules/ip6/datapath/ndp_ns_input.c b/modules/ip6/datapath/ndp_ns_input.c
@@ -87,7 +87,7 @@ static uint16_t ndp_ns_input_process(
 		}
 
 		lladdr_found = icmp6_get_opt(
-			PAYLOAD(ns), rte_pktmbuf_pkt_len(mbuf), ICMP6_OPT_SRC_LLADDR, &lladdr
+			mbuf, sizeof(*icmp6) + sizeof(*ns), ICMP6_OPT_SRC_LLADDR, &lladdr
 		);
 
 		if (rte_ipv6_addr_is_unspec(&src)) {