Skip to content

Commit

Permalink
src: Fix some memory leaks (#297)
Browse files Browse the repository at this point in the history
Fix some memory leaks

    * generate.c: free the options context allocated at the start of the main
      function.

    * openvswitch.c: make sure the GString "cmds" is free'd before the
      function returns.

    * parse.c: free the temporary list "values" before the function
      returns.

    * parse.c: parsed_defs is set to NULL preventing
      netplan_parser_reset() to free the memory allocated for the
      hash table. Not setting it to NULL is safe because all the data is moved to
      another hash table, leaving parsed_defs with no elements. Not
      nullifying parsed_defs here allows netplan_parser_reset() to free
      it.

    * types.c: in this particular case we g_strdup both key and value
      strings when inserting them in the hash table in
      parse.c:handle_generic_map(). Calling the destructor for both of
      them will make sure all the memory is free'd. When using
      free_hashtable_with_destructor in the future we'll need to make
      sure both key and value are not poiting to the same place to avoid
      a double free.

    * util.c: release the glob_t allocated in find_yaml_glob().

    * error.c: make sure the pointer 'line' is free'd after the last
      loop iteration. Also, free error_context before returning.

    * generate.c: free the error object if an error is returned at some point.

    * parse-nm.c: free "ap" when ssid can't be found (caught by clang
      build-scan).

    * parse.c: filepath might be overwritten in some situations. Make
      sure the old one is free'd.

    * parse.c: netdef->vxlan might be overwritten in some situations.
      Make sure the old one is free'd.

    * types.c: free embedded_switch_mode when the netdef is reset.
  • Loading branch information
daniloegea committed Nov 21, 2022
1 parent 5afb97a commit 26ea3a6
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/error.c
Expand Up @@ -59,6 +59,7 @@ get_syntax_error_context(const NetplanParser* npp, const int line_num, const int
line = g_data_input_stream_read_line(stream, &len, NULL, error);
}
g_string_append_printf(message, "%s\n", line);
g_free(line);

write_error_marker(message, column);

Expand Down Expand Up @@ -170,6 +171,7 @@ yaml_error(const NetplanParser *npp, const yaml_node_t* node, GError** error, co
}
g_free(s);
va_end(argp);
g_free(error_context);
return FALSE;
}

3 changes: 3 additions & 0 deletions src/generate.c
Expand Up @@ -332,6 +332,9 @@ int main(int argc, char** argv)
}

cleanup:
g_option_context_free(opt_context);
if (error)
g_error_free(error);
if (npp)
netplan_parser_clear(&npp);
if (np_state)
Expand Down
5 changes: 2 additions & 3 deletions src/openvswitch.c
Expand Up @@ -319,7 +319,7 @@ write_ovs_bridge_controller_targets(const NetplanOVSSettings* settings, const Ne
gboolean
netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinition* def, const char* rootdir, gboolean* has_been_written, GError** error)
{
GString* cmds = g_string_new(NULL);
g_autoptr(GString) cmds = g_string_new(NULL);
gchar* dependency = NULL;
const char* type = netplan_type_to_table_name(def->type);
g_autofree char* base_config_path = NULL;
Expand Down Expand Up @@ -371,7 +371,7 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio
/* Set controller target addresses */
if (def->ovs_settings.controller.addresses && def->ovs_settings.controller.addresses->len > 0) {
if (!write_ovs_bridge_controller_targets(settings, &(def->ovs_settings.controller), def->id, cmds, error))
return FALSE;
return FALSE;

/* Set controller connection mode, only applicable if at least one controller target address was set */
if (def->ovs_settings.controller.connection_mode) {
Expand Down Expand Up @@ -449,7 +449,6 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio
gboolean ret = TRUE;
if (cmds->len > 0)
ret = write_ovs_systemd_unit(def->id, cmds, rootdir, netplan_type_is_physical(def->type), FALSE, dependency, error);
g_string_free(cmds, TRUE);
SET_OPT_OUT_PTR(has_been_written, TRUE);
return ret;
}
Expand Down
1 change: 1 addition & 0 deletions src/parse-nm.c
Expand Up @@ -704,6 +704,7 @@ netplan_parser_load_keyfile(NetplanParser* npp, const char* filename, GError** e
ap->ssid = g_key_file_get_string(kf, "wifi", "ssid", NULL);
if (!ap->ssid) {
g_warning("netplan: Keyfile: cannot find SSID for WiFi connection");
g_free(ap);
return FALSE;
} else
_kf_clear_key(kf, "wifi", "ssid");
Expand Down
15 changes: 10 additions & 5 deletions src/parse.c
Expand Up @@ -2570,16 +2570,17 @@ handle_ovs_backend(NetplanParser* npp, yaml_node_t* node, const char* key_prefix
GList *external_ids = g_list_find_custom(values, "external-ids", (GCompareFunc) strcmp);
/* Non-bond/non-bridge interfaces might still be handled by the networkd backend */
if (len == 1 && (other_config || external_ids))
return ret;
goto cleanup;
else if (len == 2 && other_config && external_ids)
return ret;
goto cleanup;
}
g_list_free_full(values, g_free);

/* Set the renderer for this device to NETPLAN_BACKEND_OVS, implicitly.
* But only if empty "openvswitch: {}" or "openvswitch:" with more than
* "other-config" or "external-ids" keys is given. */
npp->current.netdef->backend = NETPLAN_BACKEND_OVS;
cleanup:
g_list_free_full(values, g_free);
return ret;
}

Expand Down Expand Up @@ -2935,8 +2936,11 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi
} else {
npp->current.netdef = netplan_netdef_new(npp, scalar(key), GPOINTER_TO_UINT(data), npp->current.backend);
}
if (npp->current.filepath)
if (npp->current.filepath) {
if (npp->current.netdef->filepath)
g_free(npp->current.netdef->filepath);
npp->current.netdef->filepath = g_strdup(npp->current.filepath);
}

// XXX: breaks multi-pass parsing.
//if (!g_hash_table_add(ids_in_file, npp->current.netdef->id))
Expand Down Expand Up @@ -2966,6 +2970,8 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi
NetplanVxlan* vxlan = g_new0(NetplanVxlan, 1);
reset_vxlan(vxlan);
npp->current.vxlan = vxlan;
if (npp->current.netdef->vxlan)
g_free(npp->current.netdef->vxlan);
npp->current.netdef->vxlan = vxlan;
}

Expand Down Expand Up @@ -3234,7 +3240,6 @@ netplan_state_import_parser_results(NetplanState* np_state, NetplanParser* npp,
/* We need to reset those fields manually as we transfered ownership of the underlying
data to out. If we don't do this, netplan_clear_parser will deallocate data
that we don't own anymore. */
npp->parsed_defs = NULL;
npp->ordered = NULL;
memset(&npp->global_ovs_settings, 0, sizeof(NetplanOVSSettings));

Expand Down
5 changes: 4 additions & 1 deletion src/types.c
Expand Up @@ -50,8 +50,10 @@ free_hashtable_with_destructor(GHashTable** hash, void (destructor)(void *)) {
GHashTableIter iter;
gpointer key, value;
g_hash_table_iter_init(&iter, *hash);
while (g_hash_table_iter_next(&iter, &key, &value))
while (g_hash_table_iter_next(&iter, &key, &value)) {
destructor(key);
destructor(value);
}
g_hash_table_destroy(*hash);
*hash = NULL;
}
Expand Down Expand Up @@ -346,6 +348,7 @@ reset_netdef(NetplanNetDefinition* netdef, NetplanDefType new_type, NetplanBacke
netdef->sriov_link = NULL;
netdef->sriov_vlan_filter = FALSE;
netdef->sriov_explicit_vf_count = G_MAXUINT; /* 0 is a valid number of VFs */
FREE_AND_NULLIFY(netdef->embedded_switch_mode);

reset_ovs_settings(&netdef->ovs_settings);
reset_backend_settings(&netdef->backend_settings, backend);
Expand Down
5 changes: 4 additions & 1 deletion src/util.c
Expand Up @@ -606,8 +606,11 @@ netplan_parser_load_yaml_hierarchy(NetplanParser* npp, const char* rootdir, GErr
config_keys = g_list_sort(g_hash_table_get_keys(configs), (GCompareFunc) strcmp);

for (GList* i = config_keys; i != NULL; i = i->next)
if (!netplan_parser_load_yaml(npp, g_hash_table_lookup(configs, i->data), error))
if (!netplan_parser_load_yaml(npp, g_hash_table_lookup(configs, i->data), error)) {
globfree(&gl);
return FALSE;
}
globfree(&gl);
return TRUE;
}

Expand Down

0 comments on commit 26ea3a6

Please sign in to comment.