On Fri, Mar 20, 2026 at 8:43 AM Eelco Chaudron <[email protected]> wrote:
>
>
> On 17 Mar 2026, at 19:55, Mike Pattrick via dev wrote:
>
> > When a batch is full, dp_packet_batch_add() frees the packet instead of
> > adding it to the batch. In dp_packet_gso__(), the code calls
> > dp_packet_batch_add() to add the original packet 'p' back into the
> > batch, but does not check if the packet was freed. If the packet was
> > freed, subsequent operations on 'p' (such as dp_packet_tunnel(),
> > dp_packet_get_inner_tcp_payload(), and dp_packet_set_size()) cause
> > undefined behavior.
> >
> > Fix by checking the return value and exiting early if the packet was
> > freed.
> >
> > Found with clang analyze.
> >
> > Fixes: ef762327f6d3 ("dp-packet-gso: Refactor software segmentation
> code.")
> > Signed-off-by: Mike Pattrick <[email protected]>
> > ---
> > lib/dp-packet-gso.c | 4 +++-
> > lib/dp-packet.h | 12 +++++++-----
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
> > index bceb851fb..0d4d52daf 100644
> > --- a/lib/dp-packet-gso.c
> > +++ b/lib/dp-packet-gso.c
> > @@ -199,7 +199,9 @@ dp_packet_gso__(struct dp_packet *p, struct
> dp_packet_batch **batches,
> > if (dp_packet_batch_is_full(curr_batch)) {
> > curr_batch++;
> > }
> > - dp_packet_batch_add(curr_batch, p);
>
> Hi Mike,
>
> Thanks for the patch. However, I don't believe this change is necessary.
>
> Just one line above the proposed change, the code checks if the batch is
> full via dp_packet_batch_is_full(curr_batch). If it is, it moves to the
> next (empty) batch in the array.
>
> This appears to be a false positive from the static analyzer (I have also
> marked this as a false positive in Coverity previously). Furthermore, if
> we were to adopt this check, it is missing from the other invocations of
> dp_packet_batch_add() later in the same function.
>
I do agree it's a false positive, however, personally I think checking the
return code is fairly light touch. If not desired, we could squash this
patch.
-M
>
> Cheers,
>
> Eelco
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
