On 6/3/24 22:56, Mark Michelson wrote:
> Like Ilya, I'm surprised that ASAN did not catch this.
>
> Having said that,
> Acked-by: Mark Michelson <[email protected]>
>
Thanks, Ales, Ilya, Mark! I applied this to main with a small change,
see below.
I also backported it to all supported branches.
> On 5/10/24 06:57, Ales Musil wrote:
>> The bitmap used in the update_ct_zones was uninitialized, and it could
>> contain any value besides 0. Use the bitmap_allocate() function instead,
>> to allocate the bitmap in heap rather than stack, the allocate makes sure
>> that the memory is properly zeroed.
>> This was caught by valgrind:
>>
>> Conditional jump or move depends on uninitialised value(s)
>> at 0x44074B: update_ct_zones (ovn-controller.c:812)
>> by 0x440DC9: en_ct_zones_run (ovn-controller.c:2579)
>> by 0x468BB7: engine_recompute (inc-proc-eng.c:415)
>> by 0x46954C: engine_compute (inc-proc-eng.c:454)
>> by 0x46954C: engine_run_node (inc-proc-eng.c:503)
>> by 0x46954C: engine_run (inc-proc-eng.c:528)
>> by 0x40AE9D: main (ovn-controller.c:5776)
>> Uninitialised value was created by a stack allocation
>> at 0x440313: update_ct_zones (ovn-controller.c:747)
>>
>> Fixes: f9cab11d5fab ("Allow explicit setting of the SNAT zone on a
>> gateway router.")
>> Signed-off-by: Ales Musil <[email protected]>
>> ---
>> v2: Use bitmap_allocate() instead of array on stack.
>> ---
>> controller/ovn-controller.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index 453dc62fd..8ee2da2fd 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -732,7 +732,7 @@ update_ct_zones(const struct sset *local_lports,
>> const char *user;
>> struct sset all_users = SSET_INITIALIZER(&all_users);
>> struct simap req_snat_zones = SIMAP_INITIALIZER(&req_snat_zones);
>> - unsigned long unreq_snat_zones_map[BITMAP_N_LONGS(MAX_CT_ZONES)];
>> + unsigned long *unreq_snat_zones_map = bitmap_allocate(MAX_CT_ZONES);
>> struct simap unreq_snat_zones =
>> SIMAP_INITIALIZER(&unreq_snat_zones);
>> const char *local_lport;
>> @@ -843,6 +843,7 @@ update_ct_zones(const struct sset *local_lports,
>> simap_destroy(&req_snat_zones);
>> simap_destroy(&unreq_snat_zones);
>> sset_destroy(&all_users);
>> + free(unreq_snat_zones_map);
bitmap_free(unreq_snat_zones_map);
>> }
>> static void
>
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
