On 5/1/25 9:45 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 > memcpy is used to limit the memory range accessed. > > Reported-at: https://issues.redhat.com/browse/FDP-1122 > Signed-off-by: Mike Pattrick <m...@redhat.com> > --- > v2: > - Changed custom functions to memcpy > - Switched to use hdr struct member. > --- > lib/netdev-offload-dpdk.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index 6ca271489..1634a86af 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -607,17 +607,15 @@ 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; > + ovs_be32 spec_vni = 0, mask_vni = 0; > > 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)); > + memcpy(&spec_vni, vxlan_spec->hdr.vni, sizeof > vxlan_spec->hdr.vni); > + memcpy(&mask_vni, vxlan_mask->hdr.vni, sizeof > vxlan_mask->hdr.vni);
Hi, Mike. My original suggestion in v1 was to either use memcpy or switch to the hdr. And since we should be moving to hdr, we should be able to just copy the whole 4 bytes of vx_vni without need for memcpy. Copying 4 bytes should also be more efficient for the CPU to do. The hdr struct is packed, but the value is actually 32-bit aligned already AFAICT, so we may even get away with a simple assignment. If not, then get_16aligned_be32() (16-byte alignment is usually guaranteed for network headers) would still be more efficient then copying 3 bytes one by one. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
