Looks good to me Acked-by: Mark Michelson <[email protected]>
On 2/21/20 6:59 AM, Dumitru Ceara wrote:
Calls to ip_address_and_port_from_lb_key() could fail parsing the 'key' argument and would return without setting *ip_address. The code in ovn_lb_create() was passing an unitialized 'backend_ip' pointer and using it unconditionally afterwards. With CFLAGS="-O3" gcc reports this issue too: $ ./configure CFLAGS="-O3" --enable-Werror [...] $ make [...] northd/ovn-northd.c: In function ‘ovnnb_db_run.isra.65’: northd/ovn-northd.c:3116:14: error: ‘backend_port’ may be used uninitialized in this function [-Werror=maybe-uninitialized] uint32_t hash = service_port; ^ northd/ovn-northd.c:3207:22: note: ‘backend_port’ was declared here uint16_t backend_port; ^ In file included from northd/ovn-northd.c:27:0: /home/dceara/git-repos/ovs/lib/hash.h:342:5: error: ‘backend_ip’ may be used uninitialized in this function [-Werror=maybe-uninitialized] return hash_bytes(s, strlen(s), basis); ^ northd/ovn-northd.c:3206:19: note: ‘backend_ip’ was declared here char *backend_ip; Fix ip_address_and_port_from_lb_key() and make it return true if parsing was successful. Signed-off-by: Dumitru Ceara <[email protected]> --- northd/ovn-northd.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 721cb05..3aba048 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -2253,7 +2253,7 @@ join_logical_ports(struct northd_context *ctx, } }-static void+static bool ip_address_and_port_from_lb_key(const char *key, char **ip_address, uint16_t *port, int *addr_family);@@ -2272,13 +2272,12 @@ get_router_load_balancer_ips(const struct ovn_datapath *od, SMAP_FOR_EACH (node, vips) {/* node->key contains IP:port or just IP. */ - char *ip_address = NULL; + char *ip_address; uint16_t port; int addr_family;- ip_address_and_port_from_lb_key(node->key, &ip_address, &port,- &addr_family); - if (!ip_address) { + if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, + &addr_family)) { continue; }@@ -3156,13 +3155,12 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs,size_t n_vips = 0;SMAP_FOR_EACH (node, &nbrec_lb->vips) {- char *vip = NULL; + char *vip; uint16_t port; int addr_family;- ip_address_and_port_from_lb_key(node->key, &vip, &port,- &addr_family); - if (!vip) { + if (!ip_address_and_port_from_lb_key(node->key, &vip, &port, + &addr_family)) { continue; }@@ -3206,10 +3204,9 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs,char *backend_ip; uint16_t backend_port;- ip_address_and_port_from_lb_key(token, &backend_ip, &backend_port,- &addr_family); - - if (!backend_ip) { + if (!ip_address_and_port_from_lb_key(token, &backend_ip, + &backend_port, + &addr_family)) { continue; }@@ -4682,8 +4679,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) /* For a 'key' of the form "IP:port" or just "IP", sets 'port' and* 'ip_address'. The caller must free() the memory allocated for - * 'ip_address'. */ -static void + * 'ip_address'. + * Returns true if parsing of 'key' was successful, false otherwise. + */ +static bool ip_address_and_port_from_lb_key(const char *key, char **ip_address, uint16_t *port, int *addr_family) { @@ -4692,16 +4691,18 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address, static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", key); - return; + *ip_address = NULL; + *port = 0; + *addr_family = 0; + return false; }struct ds s = DS_EMPTY_INITIALIZER;ss_format_address_nobracks(&ss, &s); *ip_address = ds_steal_cstr(&s); - *port = ss_get_port(&ss); - *addr_family = ss.ss_family; + return true; }/*
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
