On 10/3/25 9:37 PM, Mike Pattrick wrote:
> On Fri, Oct 3, 2025 at 2:19 PM Ilya Maximets <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 9/19/25 9:27 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
> > use ovs_16aligned_be32 to access the field as a 32bit integer.
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-1122
> <https://issues.redhat.com/browse/FDP-1122>
> > Signed-off-by: Mike Pattrick <[email protected] <mailto:[email protected]>>
> > ---
> > v3:
> > - Change from memcpy to ovs_16aligned_be32
> > v4:
> > - Correct outdated commit message
> > ---
> > lib/netdev-offload-dpdk.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index 16f863284..82461fbd9 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -607,17 +607,16 @@ 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;
> >
> > 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));
> > + ovs_be32 spec_vni = get_16aligned_be32(
> > + &ALIGNED_CAST(struct vxlanhdr *, vxlan_spec)->vx_vni);
>
> Why casting to the struct vxlanhdr and not just use the hdr.vx_vni
> that is already in the rte_flow_item_vxlan?
>
>
> Accessing that as a pointer resulted in a different compiler warning for
> unaligned pointer value in a packed struct.
Uff, that's annoying. Yeah, there seems to be no good way of getting
to vx_vni inside the packed struct without a cast...
Let's keep it this way, but I think we need to cast '&vxlan_spec->hdr'
instead of just 'vxlan_spec'. There is no guarantee that the item
structure 'rte_flow_item_vxlan' will not have any other members. While
'hdr' is more or less an actual packet header struct.
Best regards, Ilya Maximets.
>
> -M
>
>
>
> > + ovs_be32 mask_vni = get_16aligned_be32(
> > + &ALIGNED_CAST(struct vxlanhdr *, vxlan_mask)->vx_vni);
> > DUMP_PATTERN_ITEM(vxlan_mask->vni, false, "vni", "%"PRIu32,
> > ntohl(spec_vni) >> 8, ntohl(mask_vni) >>
> 8, 0);
> > }
> > @@ -695,8 +694,8 @@ dump_vxlan_encap(struct ds *s, const struct
> rte_flow_item *items)
> > if (vxlan) {
> > ovs_be32 vni;
> >
> > - vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *,
> > - vxlan->vni));
> > + vni = get_16aligned_be32(&ALIGNED_CAST(struct vxlanhdr *,
> > + vxlan)->vx_vni);
> > ds_put_format(s, "vni %"PRIu32" ", ntohl(vni) >> 8);
> > }
> > if (udp) {
> > @@ -1300,9 +1299,9 @@ 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),
> > + put_16aligned_be32(&ALIGNED_CAST(struct vxlanhdr *,
> vx_spec)->vx_vni,
> > htonl(ntohll(match->flow.tunnel.tun_id) << 8));
> > - put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_mask->vni),
> > + put_16aligned_be32(&ALIGNED_CAST(struct vxlanhdr *,
> vx_mask)->vx_vni,
> > htonl(ntohll(match->wc.masks.tunnel.tun_id) <<
> 8));
> >
> > consumed_masks->tunnel.tun_id = 0;
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
