[ovs-git] [openvswitch/ovs] 06b0ba: ovsdb-idl: Fix memleak when reinserting tracked or...

Dumitru Ceara noreply at github.com
Thu Dec 3 17:59:47 UTC 2020


  Branch: refs/heads/branch-2.14
  Home:   https://github.com/openvswitch/ovs
  Commit: 06b0ba9686aaf2a69740b251444d1335fd184fb3
      https://github.com/openvswitch/ovs/commit/06b0ba9686aaf2a69740b251444d1335fd184fb3
  Author: Dumitru Ceara <dceara at redhat.com>
  Date:   2020-12-03 (Thu, 03 Dec 2020)

  Changed paths:
    M lib/ovsdb-idl.c
    M tests/ovsdb-idl.at

  Log Message:
  -----------
  ovsdb-idl: Fix memleak when reinserting tracked orphan rows.

Considering the following updates processed by an IDL client:
1. Delete row R1 from table A while R1 is also referenced by row R2 from
   table B:
   - because row R2 still refers to row R1, this will create an orphan
     R1 but also sets row->tracked_old_datum to report to the IDL client
     that the row has been deleted.
2. Insert row R1 to table A.
   - because orphan R1 already existed in the IDL, it will be reused.
   - R1 still has row->tracked_old_datum set (and may also be on the
     table->track_list).
3. Delete row R2 from table B and row R1 from table A.
   - row->tracked_old_datum is set again but the previous
     tracked_old_datum was never freed.

IDL clients use the deleted old_datum values so when multiple delete
operations are received for a row, always track the first one as that
will match the contents of the row the IDL client knew about.

Running the newly added test case with valgrind, without the fix,
produces the following report:

==23113== 327 (240 direct, 87 indirect) bytes in 1 blocks are definitely lost in loss record 43 of 43
==23113==    at 0x4C29F73: malloc (vg_replace_malloc.c:309)
==23113==    by 0x476761: xmalloc (util.c:138)
==23113==    by 0x45D8B3: ovsdb_idl_insert_row (ovsdb-idl.c:3431)
==23113==    by 0x45B7F9: ovsdb_idl_process_update2 (ovsdb-idl.c:2670)
==23113==    by 0x45AFCF: ovsdb_idl_db_parse_update__ (ovsdb-idl.c:2479)
==23113==    by 0x45B262: ovsdb_idl_db_parse_update (ovsdb-idl.c:2542)
==23113==    by 0x45ABBE: ovsdb_idl_db_parse_update_rpc (ovsdb-idl.c:2358)
==23113==    by 0x4576DD: ovsdb_idl_process_msg (ovsdb-idl.c:865)
==23113==    by 0x457973: ovsdb_idl_run (ovsdb-idl.c:944)
==23113==    by 0x40B7B9: do_idl (test-ovsdb.c:2523)
==23113==    by 0x44425D: ovs_cmdl_run_command__ (command-line.c:247)
==23113==    by 0x44430E: ovs_cmdl_run_command (command-line.c:278)
==23113==    by 0x404BA6: main (test-ovsdb.c:76)

Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted rows.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: f48288aa4b84928cdac54a8891a9ff072dc0613b
      https://github.com/openvswitch/ovs/commit/f48288aa4b84928cdac54a8891a9ff072dc0613b
  Author: Dumitru Ceara <dceara at redhat.com>
  Date:   2020-12-03 (Thu, 03 Dec 2020)

  Changed paths:
    M lib/ovsdb-idl.c

  Log Message:
  -----------
  ovsdb-idl: Fix memleak when deleting orphan rows.

Pure IDL orphan rows, i.e., for which no "insert" operation was seen,
which are part of tables with change tracking enabled should also be
freed when the table track_list is flushed.

Reported-by: Ilya Maximets <i.maximets at ovn.org>
Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted rows.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: ef90b8f5b4569057850ac20bbe6c9a47267d32d2
      https://github.com/openvswitch/ovs/commit/ef90b8f5b4569057850ac20bbe6c9a47267d32d2
  Author: Dumitru Ceara <dceara at redhat.com>
  Date:   2020-12-03 (Thu, 03 Dec 2020)

  Changed paths:
    M lib/ovsdb-idl.c

  Log Message:
  -----------
  ovsdb-idl: Fix use-after-free when deleting orphaned rows.

It's possible that the IDL client processes multiple jsonrpc updates
in a single ovsdb_idl_run().

Considering the following updates processed in a single IDL run:
1. Update row R1 from table A while R1 is also referenced by row R2 from
   table B:
   - this adds R1 to table A's track_list.
2. Delete row R1 from table A while R1 is also referenced by row R2 from
   table B:
   - because row R2 still refers to row R1, this will create an orphan
     R1.
   - at this point R1 is still in table A's hmap.

When the IDL client calls ovsdb_idl_track_clear() after it has finished
processing the tracked changes, row R1 gets freed leaving a dangling
pointer in table A's hmap.

To fix this we don't free rows in ovsdb_idl_track_clear() if they are
orphan and still referenced by other rows, i.e., the row's 'dst_arcs'
list is not empty.  Later, when all arc sources (e.g., R2) are
deleted, the orphan R1 will be cleaned up as well.

The only exception is when the whole contents of the IDL are flushed,
in ovsdb_idl_db_clear(), in which case it's safe to free all rows.

Reported-by: Ilya Maximets <i.maximets at ovn.org>
Fixes: 932104f483ef ("ovsdb-idl: Add support for change tracking.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/0c17c9b61105...ef90b8f5b456


More information about the git mailing list