[ovs-git] [openvswitch/ovs] 43e32c: ovsdb-server: Fix schema leak while reading db.

Eiichi Tsukata noreply at github.com
Thu May 28 16:59:33 UTC 2020


  Branch: refs/heads/branch-2.9
  Home:   https://github.com/openvswitch/ovs
  Commit: 43e32cb765300aa1c03afdde31872d0c7dd0c9ad
      https://github.com/openvswitch/ovs/commit/43e32cb765300aa1c03afdde31872d0c7dd0c9ad
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2020-05-28 (Thu, 28 May 2020)

  Changed paths:
    M ovsdb/ovsdb-server.c

  Log Message:
  -----------
  ovsdb-server: Fix schema leak while reading db.

parse_txn() function doesn't always take ownership of the 'schema'
passed.  So, if the schema of the clustered db has same version as the
one that already in use, parse_txn() will not use it, resulting with a
memory leak:

 7,827 (56 direct, 7,771 indirect) bytes in 1 blocks are definitely lost
    at 0x483BB1A: calloc (vg_replace_malloc.c:762)
    by 0x44AD02: xcalloc (util.c:121)
    by 0x40E70E: ovsdb_schema_create (ovsdb.c:41)
    by 0x40EA6D: ovsdb_schema_from_json (ovsdb.c:217)
    by 0x415EDD: ovsdb_storage_read (storage.c:280)
    by 0x408968: read_db (ovsdb-server.c:607)
    by 0x40733D: main_loop (ovsdb-server.c:227)
    by 0x40733D: main (ovsdb-server.c:469)

While we could put ovsdb_schema_destroy() in a few places inside
'parse_txn()', from the users' point of view it seems better to have a
constant argument and just clone the 'schema' if needed.  The caller
will be responsible for destroying the 'schema' it owns.

Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.")
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 7362b43665af52c47fc4051cc84b1fa7a5bd2b33
      https://github.com/openvswitch/ovs/commit/7362b43665af52c47fc4051cc84b1fa7a5bd2b33
  Author: Eiichi Tsukata <eiichi.tsukata at nutanix.com>
  Date:   2020-05-28 (Thu, 28 May 2020)

  Changed paths:
    M lib/classifier.c
    M lib/classifier.h
    M tests/test-classifier.c

  Log Message:
  -----------
  classifier: Prevent tries vs n_tries race leading to NULL dereference.

Currently classifier tries and n_tries can be updated not atomically,
there is a race condition which can lead to NULL dereference.
The race can happen when main thread updates a classifier tries and
n_tries in classifier_set_prefix_fields() and at the same time revalidator
or handler thread try to lookup them in classifier_lookup__(). Such race
can be triggered when user changes prefixes of flow_table.

Race(user changes flow_table prefixes: ip_dst,ip_src => none):

  [main thread]             [revalidator/handler thread]
  ===========================================================
                            /* cls->n_tries == 2 */
                            for (int i = 0; i < cls->n_tries; i++) {
  trie_init(cls, i, NULL);
  /* n_tries == 0 */
  cls->n_tries = n_tries;
                            /* cls->tries[i]->feild is NULL */
                            trie_ctx_init(&trie_ctx[i],&cls->tries[i]);
                            /* trie->field is NULL */
                            ctx->be32ofs = trie->field->flow_be32ofs;

To prevent the race, instead of re-introducing internal mutex
implemented in the commit fccd7c092e09 ("classifier: Remove internal
mutex."), this patch makes trie field RCU protected and checks it after
read.

Fixes: fccd7c092e09 ("classifier: Remove internal mutex.")
Signed-off-by: Eiichi Tsukata <eiichi.tsukata at nutanix.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/fac520b04a0d...7362b43665af


More information about the git mailing list