Currently while handling the 24 bit VNI field in VXLAN code, netdev-offload-dpdk will actually read and write 32 bits. The byte that is overwritten is reserved and supposed to be set to zero anyways, so this is mostly harmless.
However, Openscanhub correctly identified this as a buffer overrun. Now two new functions are added for using a 32 bit integer to set these fields without overwriting a 4th byte. Reported-at: https://issues.redhat.com/browse/FDP-1122 Signed-off-by: Mike Pattrick <m...@redhat.com> --- include/openvswitch/types.h | 1 + lib/netdev-offload-dpdk.c | 23 +++++++++-------------- lib/unaligned.h | 21 +++++++++++++++++++++ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/include/openvswitch/types.h b/include/openvswitch/types.h index 8c5ec94a6..a364f024a 100644 --- a/include/openvswitch/types.h +++ b/include/openvswitch/types.h @@ -36,6 +36,7 @@ extern "C" { /* The ovs_be<N> types indicate that an object is in big-endian, not * native-endian, byte order. They are otherwise equivalent to uint<N>_t. */ typedef uint16_t OVS_BITWISE ovs_be16; +typedef uint8_t ovs_be24[3]; typedef uint32_t OVS_BITWISE ovs_be32; typedef uint64_t OVS_BITWISE ovs_be64; diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 6ca271489..66ae23f28 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -607,19 +607,17 @@ dump_flow_pattern(struct ds *s, } else if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN) { const struct rte_flow_item_vxlan *vxlan_spec = item->spec; const struct rte_flow_item_vxlan *vxlan_mask = item->mask; - ovs_be32 spec_vni, mask_vni; + uint32_t spec_vni, mask_vni; ds_put_cstr(s, "vxlan "); if (vxlan_spec) { if (!vxlan_mask) { vxlan_mask = &rte_flow_item_vxlan_mask; } - spec_vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *, - vxlan_spec->vni)); - mask_vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *, - vxlan_mask->vni)); + spec_vni = get_unaligned_be24(vxlan_spec->vni); + mask_vni = get_unaligned_be24(vxlan_mask->vni); DUMP_PATTERN_ITEM(vxlan_mask->vni, false, "vni", "%"PRIu32, - ntohl(spec_vni) >> 8, ntohl(mask_vni) >> 8, 0); + spec_vni, mask_vni, 0); } ds_put_cstr(s, "/ "); } else if (item->type == RTE_FLOW_ITEM_TYPE_GRE) { @@ -693,11 +691,10 @@ dump_vxlan_encap(struct ds *s, const struct rte_flow_item *items) ds_put_format(s, "set vxlan ip-version %s ", ipv4 ? "ipv4" : ipv6 ? "ipv6" : "ERR"); if (vxlan) { - ovs_be32 vni; + uint32_t vni; - vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *, - vxlan->vni)); - ds_put_format(s, "vni %"PRIu32" ", ntohl(vni) >> 8); + vni = get_unaligned_be24(vxlan->vni); + ds_put_format(s, "vni %"PRIu32" ", vni); } if (udp) { ds_put_format(s, "udp-src %"PRIu16" udp-dst %"PRIu16" ", @@ -1300,10 +1297,8 @@ parse_vxlan_match(struct flow_patterns *patterns, vx_spec = xzalloc(sizeof *vx_spec); vx_mask = xzalloc(sizeof *vx_mask); - put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_spec->vni), - htonl(ntohll(match->flow.tunnel.tun_id) << 8)); - put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_mask->vni), - htonl(ntohll(match->wc.masks.tunnel.tun_id) << 8)); + put_unaligned_be24(vx_spec->vni, ntohll(match->flow.tunnel.tun_id)); + put_unaligned_be24(vx_mask->vni, ntohll(match->wc.masks.tunnel.tun_id)); consumed_masks->tunnel.tun_id = 0; consumed_masks->tunnel.flags = 0; diff --git a/lib/unaligned.h b/lib/unaligned.h index 15334e3c7..212353e5f 100644 --- a/lib/unaligned.h +++ b/lib/unaligned.h @@ -31,9 +31,11 @@ static inline void put_unaligned_u32(uint32_t *, uint32_t); static inline void put_unaligned_u64(uint64_t *, uint64_t); static inline ovs_be16 get_unaligned_be16(const ovs_be16 *); +static inline uint32_t get_unaligned_be24(const ovs_be24); static inline ovs_be32 get_unaligned_be32(const ovs_be32 *); static inline ovs_be64 get_unaligned_be64(const ovs_be64 *); static inline void put_unaligned_be16(ovs_be16 *, ovs_be16); +static inline void put_unaligned_be24(ovs_be24, uint64_t); static inline void put_unaligned_be32(ovs_be32 *, ovs_be32); static inline void put_unaligned_be64(ovs_be64 *, ovs_be64); @@ -176,6 +178,25 @@ get_unaligned_be64(const ovs_be64 *p) } #endif +/* Get and set ovs_be24. These functions are useful for 24bit values in + * network protocols, like VXLAN, and convert from and to host integers for + * convenience. */ +static inline void +put_unaligned_be24(ovs_be24 p, uint64_t x) +{ + p[0] = (x >> 16) & 0xFF; + p[1] = (x >> 8) & 0xFF; + p[2] = x & 0xFF; +} + +static inline uint32_t +get_unaligned_be24(const ovs_be24 p) +{ + return ((uint32_t) p[0] << 16) + | ((uint32_t) p[1] << 8) + | (uint32_t) p[2]; +} + /* Stores 'x' at possibly misaligned address 'p'. * * put_unaligned_u64() could be overloaded in the same way as -- 2.49.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
