[ovs-git] [openvswitch/ovs] 6187bd: ovsdb-idlc: Fix memory leak reported by Coverity.

William Tu noreply at github.com
Tue May 12 15:46:05 UTC 2020


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 6187bd3aa00dff30b7db3414381e750ecf8ab777
      https://github.com/openvswitch/ovs/commit/6187bd3aa00dff30b7db3414381e750ecf8ab777
  Author: William Tu <u9012063 at gmail.com>
  Date:   2020-05-12 (Tue, 12 May 2020)

  Changed paths:
    M ovsdb/ovsdb-idlc.in

  Log Message:
  -----------
  ovsdb-idlc: Fix memory leak reported by Coverity.

Coverity shows the following memory leak in this code pattern:

void
ovsrec_ipfix_index_set_obs_domain_id(...
{
    struct ovsdb_datum datum;
//     1. alloc_fn: Storage is returned from allocation function xmalloc.
//     2. var_assign: Assigning: key = storage returned from xmalloc(16UL).
    union ovsdb_atom *key = xmalloc(sizeof(union ovsdb_atom));

//     3. Condition n_obs_domain_id, taking false branch.
    if (n_obs_domain_id) {
        datum.n = 1;
        datum.keys = key;
        key->integer = *obs_domain_id;
    } else {
        datum.n = 0;
        datum.keys = NULL;
    }
    datum.values = NULL;
    ovsdb_idl_index_write(CONST_CAST(struct ovsdb_idl_row *, &row->head...
//     CID 1420891 (#1 of 1): Resource leak (RESOURCE_LEAK)

Fixed it by moving the xmalloc to the true branch.

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
Signed-off-by: William Tu <u9012063 at gmail.com>


  Commit: 11827c63e22e6f668379dc74260a84f68940275c
      https://github.com/openvswitch/ovs/commit/11827c63e22e6f668379dc74260a84f68940275c
  Author: William Tu <u9012063 at gmail.com>
  Date:   2020-05-12 (Tue, 12 May 2020)

  Changed paths:
    M ovsdb/ovsdb-idlc.in

  Log Message:
  -----------
  ovsdb-idlc: Fix memory leak reported by Coverity.

An exmplae pattern shown below:
void
ovsrec_ct_zone_index_set_external_ids(const struct ovsrec_ct_zone...
{
//  1. alloc_fn: Storage is returned from allocation function xmalloc.
//  2. var_assign: Assigning: datum = storage returned from xmalloc(24UL).
    struct ovsdb_datum *datum = xmalloc(sizeof(struct ovsdb_datum));

//  3. Condition external_ids, taking false branch.
    if (external_ids) {
		...
    } else {
// 	4. noescape: Resource datum is not freed or pointed-to in ovsdb_datum_init_empty.
        ovsdb_datum_init_empty(datum);
    }
// 	5. noescape: Resource datum is not freed or pointed-to in ovsdb_idl_index_write.
    ovsdb_idl_index_write(CONST_CAST(struct ovsdb_idl_row *, &row->header_),
                          &ovsrec_ct_zone_columns[OVSREC_CT_ZONE_COL_EXTERNAL_IDS],
                          datum,
                          &ovsrec_table_classes[OVSREC_TABLE_CT_ZONE]);

// CID 1420856 (#1 of 1): Resource leak (RESOURCE_LEAK)
// 6. leaked_storage: Variable datum going out of scope leaks the storage it
      points to.
Fix it by freeing the datum.

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
Signed-off-by: William Tu <u9012063 at gmail.com>


  Commit: e398275024e815b52e796fcfe350fdd0d139ebba
      https://github.com/openvswitch/ovs/commit/e398275024e815b52e796fcfe350fdd0d139ebba
  Author: William Tu <u9012063 at gmail.com>
  Date:   2020-05-12 (Tue, 12 May 2020)

  Changed paths:
    M lib/ovsdb-data.c

  Log Message:
  -----------
  ovsdb-idl: Fix NULL deref reported by Coverity.

When 'datum.values' or 'datum.keys' is NULL, some code path calling
into ovsdb_idl_txn_write__ triggers NULL deref.  An example is below:

ovsrec_open_vswitch_set_cur_cfg(const struct ovsrec_open_vswitch
{
    struct ovsdb_datum datum;
    union ovsdb_atom key;

    datum.n = 1;
    datum.keys = &key;

    key.integer = cur_cfg;
//  1. assign_zero: Assigning: datum.values = NULL.
    datum.values = NULL;
//  CID 1421356 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
//  2. var_deref_model: Passing &datum to ovsdb_idl_txn_write_clone,\
//     which dereferences null datum.values.
    ovsdb_idl_txn_write_clone(&row->header_, &ovsrec_open_vswitch_col
}

And with the following calls:
ovsdb_idl_txn_write_clone
  ovsdb_idl_txn_write__
    6. deref_parm_in_call: Function ovsdb_datum_destroy dereferences
       datum->values
	ovsdb_datum_destroy

And another possible NULL deref is at ovsdb_datum_equals(). Fix the
two by adding additional checks.

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
Signed-off-by: William Tu <u9012063 at gmail.com>


Compare: https://github.com/openvswitch/ovs/compare/732cb79fb867...e398275024e8


More information about the git mailing list