From: Alejandro Pacheco <[email protected]>
The loops that look for the SLL or TLL option in packet_set_nd() and
odp_set_nd() advance to the next option before subtracting its length,
so they subtract the length of the next option instead of the current
one. The remaining byte count goes wrong and the loop can read and
write options past the end of the packet. Subtract before advancing.
packet_set_nd_ext() and odp_set_nd_ext() also access the first option's
type when the packet holds only the ND header. Check for it first.
Fixes: e60e935b1f37 ("Implement set-field for IPv6 ND fields (nd_target,
nd_sll, and nd_tll).")
Fixes: 9b2b84973db7 ("Support for match & set ICMPv6 reserved and options type
fields")
Assisted-by: claude-opus-5-5, Claude Code
Signed-off-by: Alejandro Pacheco <[email protected]>
---
v3: Assert the L4 header in check_set_nd() before comparing the
options, for the clang-analyze NonNullParamChecker warning.
lib/odp-execute.c | 8 +++--
lib/packets.c | 6 ++--
tests/test-packets.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 618fb5ac8..3bec8ee00 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -332,7 +332,11 @@ odp_set_nd_ext(struct dp_packet *packet, const struct
ovs_key_nd_extensions
{
const struct ovs_nd_msg *ns = dp_packet_l4(packet);
ovs_16aligned_be32 reserved = ns->rso_flags;
- uint8_t opt_type = ns->options[0].type;
+ uint8_t opt_type = 0;
+
+ if (dp_packet_l4_size(packet) >= sizeof *ns + ND_LLA_OPT_LEN) {
+ opt_type = ns->options[0].type;
+ }
if (mask->nd_reserved) {
put_16aligned_be32(&reserved, key->nd_reserved);
@@ -374,8 +378,8 @@ odp_set_nd(struct dp_packet *packet, const struct
ovs_key_nd *key,
break;
}
- lla_opt += lla_opt->len;
bytes_remain -= lla_opt->len * ND_LLA_OPT_LEN;
+ lla_opt += lla_opt->len;
}
packet_set_nd(packet,
diff --git a/lib/packets.c b/lib/packets.c
index 80c41e4b6..995540824 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1652,7 +1652,9 @@ packet_set_nd_ext(struct dp_packet *packet, const
ovs_16aligned_be32 rso_flags,
/* set RSO flags and option type */
ns->rso_flags = rso_flags;
- opt->type = opt_type;
+ if (bytes_remain >= sizeof(*ns) + ND_LLA_OPT_LEN) {
+ opt->type = opt_type;
+ }
/* recalculate checksum */
ovs_be16 *csum_value = &(ns->icmph.icmp6_cksum);
@@ -1707,8 +1709,8 @@ packet_set_nd(struct dp_packet *packet, const struct
in6_addr *target,
break;
}
- opt += opt->len;
bytes_remain -= opt->len * ND_LLA_OPT_LEN;
+ opt += opt->len;
}
}
diff --git a/tests/test-packets.c b/tests/test-packets.c
index 6151c633c..8efc2cdc3 100644
--- a/tests/test-packets.c
+++ b/tests/test-packets.c
@@ -21,6 +21,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include "dp-packet.h"
#include "ovstest.h"
static void
@@ -173,6 +174,75 @@ test_ipv6_parsing(void)
assert(ipv6_count_cidr_bits(&mask) == 128);
}
+/* Checks that packet_set_nd() changes only the address of the option at
+ * 'lla_ofs' into 'opts', or nothing if 'lla_ofs' is -1. */
+static void
+check_set_nd(const uint8_t *opts, size_t n, int lla_ofs)
+{
+ static const struct eth_addr sll = ETH_ADDR_C(aa, bb, cc, dd, ee, 01);
+ static const struct eth_addr tll = ETH_ADDR_C(aa, bb, cc, dd, ee, 02);
+ size_t size = sizeof(struct ovs_nd_msg) + n;
+ struct ovs_nd_msg *ns = xzalloc(size);
+ struct in6_addr target;
+ struct dp_packet *packet;
+ uint8_t *expected;
+
+ memcpy(ns->options, opts, n);
+ memcpy(&target, &ns->target, sizeof target);
+
+ expected = xmemdup(ns->options, n);
+ if (lla_ofs >= 0) {
+ const struct eth_addr *mac = (opts[lla_ofs] == ND_OPT_SOURCE_LINKADDR
+ ? &sll : &tll);
+
+ memcpy(&expected[lla_ofs + 2], mac, sizeof *mac);
+ }
+
+ packet = dp_packet_clone_data(ns, size);
+ free(ns);
+ dp_packet_set_l4(packet, dp_packet_data(packet));
+ packet_set_nd(packet, &target, sll, tll);
+
+ ns = dp_packet_l4(packet);
+ ovs_assert(ns);
+ ovs_assert(!memcmp(ns->options, expected, n));
+
+ dp_packet_delete(packet);
+ free(expected);
+}
+
+static void
+test_set_nd(void)
+{
+ static const uint8_t sll[] = {
+ ND_OPT_SOURCE_LINKADDR, 1, 1, 2, 3, 4, 5, 6,
+ };
+ static const uint8_t mtu_tll[] = {
+ 5, 1, 0, 0, 0, 0, 5, 220,
+ ND_OPT_TARGET_LINKADDR, 1, 1, 2, 3, 4, 5, 6,
+ };
+ static const uint8_t long_short[] = {
+ 3, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 3, 1, 0, 0, 0, 0, 0, 0,
+ };
+ static const uint8_t long_sll[] = {
+ 3, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ ND_OPT_SOURCE_LINKADDR, 1, 1, 2, 3, 4, 5, 6,
+ };
+ static const uint8_t tll_sll[] = {
+ ND_OPT_TARGET_LINKADDR, 1, 1, 2, 3, 4, 5, 6,
+ ND_OPT_SOURCE_LINKADDR, 1, 1, 2, 3, 4, 5, 6,
+ };
+
+ check_set_nd(sll, 0, -1);
+ check_set_nd(sll, sizeof sll, 0);
+ check_set_nd(mtu_tll, sizeof mtu_tll, 8);
+ check_set_nd(long_short, sizeof long_short, -1);
+ check_set_nd(long_sll, sizeof long_sll, 16);
+ check_set_nd(long_sll, 16, -1);
+ check_set_nd(tll_sll, sizeof tll_sll, 0);
+}
+
static void
test_packets_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
{
@@ -181,6 +251,7 @@ test_packets_main(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
test_ipv6_cidr();
test_ipv6_masking();
test_ipv6_parsing();
+ test_set_nd();
}
OVSTEST_REGISTER("test-packets", test_packets_main);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
