On 11/28/24 4:28 PM, Ilya Maximets wrote:
> On 11/28/24 16:16, Dumitru Ceara wrote:
>> On 11/28/24 4:05 PM, Ilya Maximets wrote:
>>> Statically allocated datum objects should be properly initialized with
>>> a special function instead of doing that manually.
>>>
>>> WARNING: MemorySanitizer: use-of-uninitialized-value
>>> 0 0x58789c in ovsdb_datum_compare_3way lib/ovsdb-data.c:1846:19
>>> 1 0x52bbab in evaluate_relop lib/db-ctl-base.c:731:16
>>> 2 0x52b042 in check_condition lib/db-ctl-base.c:844:22
>>> 3 0x522fea in cmd_wait_until lib/db-ctl-base.c:1935:22
>>> 4 0x4c704b in do_vsctl utilities/ovs-vsctl.c:3001:13
>>> 5 0x4c4429 in main utilities/ovs-vsctl.c:204:17
>>> 6 0x7f5ad5 in __libc_start_call_main
>>> 7 0x7f5ad5 in __libc_start_main@GLIBC_2.2.5
>>> 8 0x432b04 in _start (utilities/ovs-vsctl+0x432b04)
>>>
>>> In this case the reference counter ended up not initialized.
>>>
>>> Fixes: 485ac63d10f8 ("ovsdb: Add lazy-copy support for ovsdb_datum
>>> objects.")
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>> ---
>>> lib/db-ctl-base.c | 6 +-----
>>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
>>> index b3e9b92d1..69b4f826c 100644
>>> --- a/lib/db-ctl-base.c
>>> +++ b/lib/db-ctl-base.c
>>> @@ -831,14 +831,10 @@ check_condition(const struct ovsdb_idl_table_class
>>> *table,
>>> } else {
>>> struct ovsdb_datum a;
>>>
>>> + ovsdb_datum_init_empty(&a);
>>> if (found) {
>>> a.n = 1;
>>> a.keys = &have_datum->values[idx];
>>> - a.values = NULL;
>>> - } else {
>>> - a.n = 0;
>>> - a.keys = NULL;
>>> - a.values = NULL;
>>> }
>>>
>>> retval = evaluate_relop(&a, &b, &type, operator);
>>
>> Hi Ilya,
>>
>> This looks OK, but checking if there are other instances I see some
>> (probably benign). Maybe we shouldn't initialize them manually either
>> though.
>>
>> - in list_record()
>
> I can fix this one as well in v2. Thanks!
>
>> - in ovsdb_datum_default()
>
> This one is tricky, it manipulates the reference counter explicitly, so
> the default datums are not getting destroyed. It is also part of the
> ovsdb-data module, so it is allowed to know and manipulate the internals.
>
> It should probably be kept as is.
>
Ack, it's probably fine. Looking forward to v2.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
