On 4/28/25 6:52 PM, Mike Pattrick via dev wrote: > 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);
There is no such thing as "unaligned be24". 24-bit value is always aligned, because it's alignment is 1 byte. So, these new helpers do not make a lot of sense. Plain memcpy should be used if necessary. However, we should just use 'hdr.vx_vni' instead of 'vni', as the non-'hdr' fields in rte_flow structures are deprecated anyway. 'vx_vni' is a 32bit field and can be accessed with the current 32bit functions. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
