On 5/10/24 10:44, Ales Musil wrote:
> The bitmap used in the update_ct_zones was uninitialized and it could
> contain any value besides 0, make sure we initialize it to 0 instead.
> 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]>
> ---
> controller/ovn-controller.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 453dc62fd..2388a1c15 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_N_LONGS(MAX_CT_ZONES)] = {0};
> struct simap unreq_snat_zones = SIMAP_INITIALIZER(&unreq_snat_zones);
>
> const char *local_lport;
Hi, Ales. Thanks for the fix!
The issue is caused by not using a proper bitmap API. Can we just use
the bitmap_allocate() here instead? With the amount of dynamic memory
allocations this function does with all the hash maps adding one more
allocation will not make any difference, but may protect from potential
issues of not using the API / providing a bad example. Allocating 8KB
on stack is not a particularly good thing anyway.
What do you think?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev