On Thu, Aug 28, 2025 at 9:09 AM Eelco Chaudron <echau...@redhat.com> wrote:
>
>
> On 28 Aug 2025, at 13:29, Mike Pattrick wrote:
>
> > On Tue, Aug 26, 2025 at 8:35 AM Eelco Chaudron via dev <
> > ovs-dev@openvswitch.org> wrote:
> >
> >> Coverity reported a possible null dereference of datum->values
> >> when handling value-type weak references in find_and_add_weak_refs().
> >>
> >> In practice, datum->values must be non-NULL for value-type weak
> references.
> >> Adding ovs_assert(datum->values) documents this invariant and ensures
> that
> >> we catch any unexpected misuse.
> >>
> >> No change to runtime behavior in production builds; this only improves
> >> safety and satisfies static analysis.
> >>
> >> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
> >> ---
> >> ovsdb/transaction.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> >> index 39650d3b3..65cfab44b 100644
> >> --- a/ovsdb/transaction.c
> >> +++ b/ovsdb/transaction.c
> >> @@ -647,6 +647,7 @@ find_and_add_weak_refs(const struct ovsdb_row *src,
> >>
> >> if (ovsdb_base_type_is_weak_ref(&column->type.value)) {
> >> for (i = 0; i < datum->n; i++) {
> >> + ovs_assert(datum->values);
> >>
> >
> > Should this be moved out of the loop?
>
> Moved it into the loop, as my thought where that if there are no entries
> (->n), the values values could be NULL as it’s not used.
>
> Maybe it could be moved outside the loop as;
>
> ovs_assert(!datum->n || datum->values);
>
> I support this.
-M
> Thoughts?
>
> //Eelco
>
> >> find_and_add_weak_ref(src, &datum->keys[i],
> &datum->values[i],
> >> column, false, ref_list, not_found,
> >> zero);
> >> }
> >> --
> >> 2.50.1
> >>
> >> _______________________________________________
> >> dev mailing list
> >> d...@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >>
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
