2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00

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: 72aeb243a5 ("ovsdb-idl: Tracking - preserve data for deleted rows.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Dumitru Ceara
2020-11-30 17:41:14 +01:00
committed by Ilya Maximets
parent fa57e9e452
commit 2407099c9f
2 changed files with 53 additions and 1 deletions

View File

@@ -3227,7 +3227,7 @@ ovsdb_idl_row_clear_old(struct ovsdb_idl_row *row)
{
ovs_assert(row->old_datum == row->new_datum);
if (!ovsdb_idl_row_is_orphan(row)) {
if (ovsdb_idl_track_is_set(row->table)) {
if (ovsdb_idl_track_is_set(row->table) && !row->tracked_old_datum) {
row->tracked_old_datum = row->old_datum;
} else {
const struct ovsdb_idl_table_class *class = row->table->class_;

View File

@@ -1225,6 +1225,58 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan weak refer
008: done
]])
dnl This test creates database with weak references and checks that the
dnl content of orphaned rows created for weak references after monitor
dnl condition change are not leaked when the row is reinserted and deleted.
OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan rows, conditional],
[['["idltest",
{"op": "insert",
"table": "simple",
"row": {"s": "row0_s"},
"uuid-name": "weak_row0"},
{"op": "insert",
"table": "simple",
"row": {"s": "row1_s"},
"uuid-name": "weak_row1"},
{"op": "insert",
"table": "simple6",
"row": {"name": "first_row",
"weak_ref": ["set",
[["named-uuid", "weak_row0"]]
]}}]']],
[['condition simple []' \
'condition simple [["s","==","row0_s"]]' \
'condition simple [["s","==","row1_s"]]' \
'condition simple [["s","==","row0_s"]]' \
'["idltest",
{"op": "delete",
"table": "simple6",
"where": []}]']],
[[000: change conditions
001: inserted row: uuid=<0>
001: name=first_row weak_ref=[] uuid=<0>
001: updated columns: name weak_ref
002: change conditions
003: i=0 r=0 b=false s=row0_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
003: inserted row: uuid=<2>
003: name=first_row weak_ref=[<2>] uuid=<0>
003: updated columns: s
004: change conditions
005: i=0 r=0 b=false s=row1_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3>
005: inserted row: uuid=<3>
005: updated columns: s
006: change conditions
007: deleted row: uuid=<3>
007: i=0 r=0 b=false s=row0_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
007: i=0 r=0 b=false s=row1_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3>
007: inserted row: uuid=<2>
007: name=first_row weak_ref=[<2>] uuid=<0>
007: updated columns: s
008: {"error":null,"result":[{"count":1}]}
009: i=0 r=0 b=false s=row0_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
010: done
]])
OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops],
[],
[['["idltest",