From ab79d262e17442926f1e480984ad663b978d46d7 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 14 Jun 2017 09:07:45 -0700 Subject: [PATCH] netlink: Introduce helpers for 128-bit integer attributes. Use the helpers in appropriate places. In most cases, this fixes a misaligned reference, since ovs_be128 and ovs_u128 require 8-byte alignment but Netlink only guarantees 4-byte. Found by GCC -fsanitize=undefined. Reported-by: Lance Richardson Signed-off-by: Ben Pfaff Acked-by: Lance Richardson --- lib/dpif-netlink.c | 12 +++-------- lib/netlink.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++ lib/netlink.h | 10 +++++++++ lib/odp-util.c | 26 +++++++++------------- 4 files changed, 77 insertions(+), 25 deletions(-) diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 71b1d5d19..351cee0b7 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -2924,8 +2924,7 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, .optional = true }, [OVS_FLOW_ATTR_TCP_FLAGS] = { .type = NL_A_U8, .optional = true }, [OVS_FLOW_ATTR_USED] = { .type = NL_A_U64, .optional = true }, - [OVS_FLOW_ATTR_UFID] = { .type = NL_A_UNSPEC, .optional = true, - .min_len = sizeof(ovs_u128) }, + [OVS_FLOW_ATTR_UFID] = { .type = NL_A_U128, .optional = true }, /* The kernel never uses OVS_FLOW_ATTR_CLEAR. */ /* The kernel never uses OVS_FLOW_ATTR_PROBE. */ /* The kernel never uses OVS_FLOW_ATTR_UFID_FLAGS. */ @@ -2957,11 +2956,7 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, } if (a[OVS_FLOW_ATTR_UFID]) { - const ovs_u128 *ufid; - - ufid = nl_attr_get_unspec(a[OVS_FLOW_ATTR_UFID], - nl_attr_get_size(a[OVS_FLOW_ATTR_UFID])); - flow->ufid = *ufid; + flow->ufid = nl_attr_get_u128(a[OVS_FLOW_ATTR_UFID]); flow->ufid_present = true; } if (a[OVS_FLOW_ATTR_MASK]) { @@ -3032,8 +3027,7 @@ dpif_netlink_flow_to_ofpbuf(const struct dpif_netlink_flow *flow, ovs_header->dp_ifindex = flow->dp_ifindex; if (flow->ufid_present) { - nl_msg_put_unspec(buf, OVS_FLOW_ATTR_UFID, &flow->ufid, - sizeof flow->ufid); + nl_msg_put_u128(buf, OVS_FLOW_ATTR_UFID, flow->ufid); } if (flow->ufid_terse) { nl_msg_put_u32(buf, OVS_FLOW_ATTR_UFID_FLAGS, diff --git a/lib/netlink.c b/lib/netlink.c index 0246131a4..4cf1aaca6 100644 --- a/lib/netlink.c +++ b/lib/netlink.c @@ -288,6 +288,14 @@ nl_msg_put_u64(struct ofpbuf *msg, uint16_t type, uint64_t value) nl_msg_put_unspec(msg, type, &value, sizeof value); } +/* Appends a Netlink attribute of the given 'type' and the given 128-bit host + * byte order 'value' to 'msg'. */ +void +nl_msg_put_u128(struct ofpbuf *msg, uint16_t type, ovs_u128 value) +{ + nl_msg_put_unspec(msg, type, &value, sizeof value); +} + /* Appends a Netlink attribute of the given 'type' and the given 16-bit network * byte order 'value' to 'msg'. */ void @@ -312,6 +320,14 @@ nl_msg_put_be64(struct ofpbuf *msg, uint16_t type, ovs_be64 value) nl_msg_put_unspec(msg, type, &value, sizeof value); } +/* Appends a Netlink attribute of the given 'type' and the given 128-bit + * network byte order 'value' to 'msg'. */ +void +nl_msg_put_be128(struct ofpbuf *msg, uint16_t type, ovs_be128 value) +{ + nl_msg_put_unspec(msg, type, &value, sizeof value); +} + /* Appends a Netlink attribute of the given 'type' and the given IPv6 * address order 'value' to 'msg'. */ void @@ -416,6 +432,14 @@ nl_msg_push_u64(struct ofpbuf *msg, uint16_t type, uint64_t value) nl_msg_push_unspec(msg, type, &value, sizeof value); } +/* Prepends a Netlink attribute of the given 'type' and the given 128-bit host + * byte order 'value' to 'msg'. */ +void +nl_msg_push_u128(struct ofpbuf *msg, uint16_t type, ovs_u128 value) +{ + nl_msg_push_unspec(msg, type, &value, sizeof value); +} + /* Prepends a Netlink attribute of the given 'type' and the given 16-bit * network byte order 'value' to 'msg'. */ void @@ -440,6 +464,14 @@ nl_msg_push_be64(struct ofpbuf *msg, uint16_t type, ovs_be64 value) nl_msg_push_unspec(msg, type, &value, sizeof value); } +/* Prepends a Netlink attribute of the given 'type' and the given 128-bit + * network byte order 'value' to 'msg'. */ +void +nl_msg_push_be128(struct ofpbuf *msg, uint16_t type, ovs_be128 value) +{ + nl_msg_push_unspec(msg, type, &value, sizeof value); +} + /* Prepends a Netlink attribute of the given 'type' and the given * null-terminated string 'value' to 'msg'. */ void @@ -625,6 +657,16 @@ nl_attr_get_u64(const struct nlattr *nla) return get_32aligned_u64(x); } +/* Returns the 128-bit host byte order value in 'nla''s payload. + * + * Asserts that 'nla''s payload is at least 16 bytes long. */ +ovs_u128 +nl_attr_get_u128(const struct nlattr *nla) +{ + const ovs_32aligned_u128 *x = nl_attr_get_unspec(nla, sizeof *x); + return get_32aligned_u128(x); +} + /* Returns the 16-bit network byte order value in 'nla''s payload. * * Asserts that 'nla''s payload is at least 2 bytes long. */ @@ -653,6 +695,16 @@ nl_attr_get_be64(const struct nlattr *nla) return get_32aligned_be64(x); } +/* Returns the 128-bit network byte order value in 'nla''s payload. + * + * Asserts that 'nla''s payload is at least 16 bytes long. */ +ovs_be128 +nl_attr_get_be128(const struct nlattr *nla) +{ + const ovs_32aligned_be128 *x = nl_attr_get_unspec(nla, sizeof *x); + return get_32aligned_be128(x); +} + /* Returns the IPv6 address value in 'nla''s payload. * * Asserts that 'nla''s payload is at least 16 bytes long. */ @@ -700,6 +752,7 @@ min_attr_len(enum nl_attr_type type) case NL_A_U16: return 2; case NL_A_U32: return 4; case NL_A_U64: return 8; + case NL_A_U128: return 16; case NL_A_STRING: return 1; case NL_A_FLAG: return 0; case NL_A_IPV6: return 16; @@ -719,6 +772,7 @@ max_attr_len(enum nl_attr_type type) case NL_A_U16: return 2; case NL_A_U32: return 4; case NL_A_U64: return 8; + case NL_A_U128: return 16; case NL_A_STRING: return SIZE_MAX; case NL_A_FLAG: return SIZE_MAX; case NL_A_IPV6: return 16; diff --git a/lib/netlink.h b/lib/netlink.h index bb4dbf661..6dfac27c9 100644 --- a/lib/netlink.h +++ b/lib/netlink.h @@ -67,9 +67,11 @@ void nl_msg_put_u8(struct ofpbuf *, uint16_t type, uint8_t value); void nl_msg_put_u16(struct ofpbuf *, uint16_t type, uint16_t value); void nl_msg_put_u32(struct ofpbuf *, uint16_t type, uint32_t value); void nl_msg_put_u64(struct ofpbuf *, uint16_t type, uint64_t value); +void nl_msg_put_u128(struct ofpbuf *, uint16_t type, ovs_u128 value); void nl_msg_put_be16(struct ofpbuf *, uint16_t type, ovs_be16 value); void nl_msg_put_be32(struct ofpbuf *, uint16_t type, ovs_be32 value); void nl_msg_put_be64(struct ofpbuf *, uint16_t type, ovs_be64 value); +void nl_msg_put_be128(struct ofpbuf *, uint16_t type, ovs_be128 value); void nl_msg_put_in6_addr(struct ofpbuf *msg, uint16_t type, const struct in6_addr *value); void nl_msg_put_odp_port(struct ofpbuf *, uint16_t type, odp_port_t value); @@ -92,9 +94,11 @@ void nl_msg_push_u8(struct ofpbuf *, uint16_t type, uint8_t value); void nl_msg_push_u16(struct ofpbuf *, uint16_t type, uint16_t value); void nl_msg_push_u32(struct ofpbuf *, uint16_t type, uint32_t value); void nl_msg_push_u64(struct ofpbuf *, uint16_t type, uint64_t value); +void nl_msg_push_u128(struct ofpbuf *, uint16_t type, ovs_u128 value); void nl_msg_push_be16(struct ofpbuf *, uint16_t type, ovs_be16 value); void nl_msg_push_be32(struct ofpbuf *, uint16_t type, ovs_be32 value); void nl_msg_push_be64(struct ofpbuf *, uint16_t type, ovs_be64 value); +void nl_msg_push_be128(struct ofpbuf *, uint16_t type, ovs_be128 value); void nl_msg_push_string(struct ofpbuf *, uint16_t type, const char *value); /* Separating buffers into individual messages. */ @@ -114,9 +118,11 @@ struct nlmsghdr *nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg); #define NL_A_U16_SIZE NL_ATTR_SIZE(sizeof(uint16_t)) #define NL_A_U32_SIZE NL_ATTR_SIZE(sizeof(uint32_t)) #define NL_A_U64_SIZE NL_ATTR_SIZE(sizeof(uint64_t)) +#define NL_A_U128_SIZE NL_ATTR_SIZE(sizeof(ovs_u128)) #define NL_A_BE16_SIZE NL_ATTR_SIZE(sizeof(ovs_be16)) #define NL_A_BE32_SIZE NL_ATTR_SIZE(sizeof(ovs_be32)) #define NL_A_BE64_SIZE NL_ATTR_SIZE(sizeof(ovs_be64)) +#define NL_A_BE128_SIZE NL_ATTR_SIZE(sizeof(ovs_be128)) #define NL_A_FLAG_SIZE NL_ATTR_SIZE(0) #define NL_A_IPV6_SIZE NL_ATTR_SIZE(sizeof(struct in6_addr)) @@ -134,6 +140,8 @@ enum nl_attr_type NL_A_BE32 = NL_A_U32, NL_A_U64, NL_A_BE64 = NL_A_U64, + NL_A_U128, + NL_A_BE128 = NL_A_U128, NL_A_STRING, NL_A_FLAG, NL_A_IPV6, @@ -195,9 +203,11 @@ uint8_t nl_attr_get_u8(const struct nlattr *); uint16_t nl_attr_get_u16(const struct nlattr *); uint32_t nl_attr_get_u32(const struct nlattr *); uint64_t nl_attr_get_u64(const struct nlattr *); +ovs_u128 nl_attr_get_u128(const struct nlattr *); ovs_be16 nl_attr_get_be16(const struct nlattr *); ovs_be32 nl_attr_get_be32(const struct nlattr *); ovs_be64 nl_attr_get_be64(const struct nlattr *); +ovs_be128 nl_attr_get_be128(const struct nlattr *); struct in6_addr nl_attr_get_in6_addr(const struct nlattr *nla); odp_port_t nl_attr_get_odp_port(const struct nlattr *); const char *nl_attr_get_string(const struct nlattr *); diff --git a/lib/odp-util.c b/lib/odp-util.c index b20c74aec..bfd3a1afc 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2894,8 +2894,8 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma, break; case OVS_KEY_ATTR_CT_LABELS: { - const ovs_u128 *value = nl_attr_get(a); - const ovs_u128 *mask = ma ? nl_attr_get(ma) : NULL; + const ovs_32aligned_u128 *value = nl_attr_get(a); + const ovs_32aligned_u128 *mask = ma ? nl_attr_get(ma) : NULL; format_u128(ds, value, mask, verbose); break; @@ -3166,16 +3166,14 @@ generate_all_wildcard_mask(const struct attr_len_tbl tbl[], int max, } static void -format_u128(struct ds *ds, const ovs_u128 *key, const ovs_u128 *mask, - bool verbose) +format_u128(struct ds *ds, const ovs_32aligned_u128 *key, + const ovs_32aligned_u128 *mask, bool verbose) { - if (verbose || (mask && !ovs_u128_is_zero(*mask))) { - ovs_be128 value; - - value = hton128(*key); + if (verbose || (mask && !ovs_u128_is_zero(get_32aligned_u128(mask)))) { + ovs_be128 value = hton128(get_32aligned_u128(key)); ds_put_hex(ds, &value, sizeof value); - if (mask && !(ovs_u128_is_ones(*mask))) { - value = hton128(*mask); + if (mask && !(ovs_u128_is_ones(get_32aligned_u128(mask)))) { + value = hton128(get_32aligned_u128(mask)); ds_put_char(ds, '/'); ds_put_hex(ds, &value, sizeof value); } @@ -4804,9 +4802,7 @@ odp_key_to_dp_packet(const struct nlattr *key, size_t key_len, wanted_attrs &= ~(1u << OVS_KEY_ATTR_CT_MARK); break; case OVS_KEY_ATTR_CT_LABELS: { - const ovs_u128 *cl = nl_attr_get(nla); - - md->ct_label = *cl; + md->ct_label = nl_attr_get_u128(nla); wanted_attrs &= ~(1u << OVS_KEY_ATTR_CT_LABELS); break; } @@ -5438,9 +5434,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_CT_MARK; } if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_CT_LABELS)) { - const ovs_u128 *cl = nl_attr_get(attrs[OVS_KEY_ATTR_CT_LABELS]); - - flow->ct_label = *cl; + flow->ct_label = nl_attr_get_u128(attrs[OVS_KEY_ATTR_CT_LABELS]); expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_CT_LABELS; } if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4)) { -- 2.39.2