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);
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
