diff options
author | Ben Pfaff <blp@ovn.org> | 2018-08-14 11:31:46 -0700 |
---|---|---|
committer | Ben Pfaff <blp@ovn.org> | 2018-08-16 10:24:05 -0700 |
commit | aee7f431df925c006d212080db9aa80c9acd0385 (patch) | |
tree | a2c4bd71d49c1b01ddbe916b196388dad6041b09 | |
parent | 92d0d515d67b9232986f71d89cafe7251e67844f (diff) | |
download | openvswitch-aee7f431df925c006d212080db9aa80c9acd0385.tar.gz |
ovsdb-idl: Adjust indexes during transactions.
When transactions modified tables with indexes, the indexes were not
properly updated to reflect the changes. For deleted rows, in particular,
this could cause use-after-free errors.
This commit fixes the problem and adds some simple test cases provided by
Han Zhou that, without the fix, cause a crash.
Reported-by: Han Zhou <zhouhan@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047185.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
-rw-r--r-- | lib/ovsdb-idl.c | 25 | ||||
-rw-r--r-- | tests/test-ovsdb.c | 17 |
2 files changed, 37 insertions, 5 deletions
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index b95f9f8fa..01a8d54ab 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -3515,9 +3515,18 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn *txn) txn->db->txn = NULL; HMAP_FOR_EACH_SAFE (row, next, txn_node, &txn->txn_rows) { + enum { INSERTED, MODIFIED, DELETED } op + = (!row->new_datum ? DELETED + : !row->old_datum ? INSERTED + : MODIFIED); + + if (op != DELETED) { + ovsdb_idl_remove_from_indexes(row); + } + ovsdb_idl_destroy_all_map_op_lists(row); ovsdb_idl_destroy_all_set_op_lists(row); - if (row->old_datum) { + if (op != INSERTED) { if (row->written) { ovsdb_idl_row_unparse(row); ovsdb_idl_row_clear_arcs(row, false); @@ -3536,7 +3545,9 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn *txn) hmap_remove(&txn->txn_rows, &row->txn_node); hmap_node_nullify(&row->txn_node); - if (!row->old_datum) { + if (op != INSERTED) { + ovsdb_idl_add_to_indexes(row); + } else { hmap_remove(&row->table->rows, &row->hmap_node); free(row); } @@ -4216,6 +4227,10 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_, goto discard_datum; } + bool index_row = is_index_row(row); + if (!index_row) { + ovsdb_idl_remove_from_indexes(row); + } if (hmap_node_is_null(&row->txn_node)) { hmap_insert(&row->table->db->txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); @@ -4238,6 +4253,9 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_, } (column->unparse)(row); (column->parse)(row, &row->new_datum[column_idx]); + if (!index_row) { + ovsdb_idl_add_to_indexes(row); + } return; discard_datum: @@ -4365,6 +4383,8 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row *row_) } ovs_assert(row->new_datum != NULL); + ovs_assert(!is_index_row(row_)); + ovsdb_idl_remove_from_indexes(row_); if (!row->old_datum) { ovsdb_idl_row_unparse(row); ovsdb_idl_row_clear_new(row); @@ -4412,6 +4432,7 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn, row->new_datum = xmalloc(class->n_columns * sizeof *row->new_datum); hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid)); hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); + ovsdb_idl_add_to_indexes(row); return row; } diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index f3f0e4967..453d88eab 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2880,9 +2880,17 @@ test_idl_compound_index_single_column(struct ovsdb_idl *idl, ovs_assert(myRow->i == 4); txn = ovsdb_idl_txn_create(idl); idltest_simple_delete(myRow); - toInsert = idltest_simple_insert(txn); - idltest_simple_set_i(toInsert, 54); - idltest_simple_set_s(toInsert, "Lista054"); + myRow = idltest_simple_index_find(i_index, toDelete); + ovs_assert(!myRow); + myRow = idltest_simple_insert(txn); + idltest_simple_set_i(myRow, 54); + idltest_simple_set_s(myRow, "Lista054"); + toInsert = idltest_simple_index_init_row(i_index); + idltest_simple_index_set_i(toInsert, 54); + myRow = idltest_simple_index_find(i_index, toInsert); + ovs_assert(myRow); + ovs_assert(myRow->i == 54); + ovs_assert(!strcmp(myRow->s, "Lista054")); ovsdb_idl_txn_commit_block(txn); ovsdb_idl_txn_destroy(txn); idltest_simple_index_set_i(to, 60); @@ -2904,6 +2912,8 @@ test_idl_compound_index_single_column(struct ovsdb_idl *idl, ovs_assert(myRow->i == 10); txn = ovsdb_idl_txn_create(idl); idltest_simple_set_i(myRow, 30); + myRow = idltest_simple_index_find(i_index, toUpdate); + ovs_assert(!myRow); ovsdb_idl_txn_commit_block(txn); ovsdb_idl_txn_destroy(txn); idltest_simple_index_set_i(to, 60); @@ -2927,6 +2937,7 @@ test_idl_compound_index_single_column(struct ovsdb_idl *idl, idltest_simple_index_destroy_row(to); idltest_simple_index_destroy_row(equal); idltest_simple_index_destroy_row(toDelete); + idltest_simple_index_destroy_row(toInsert); idltest_simple_index_destroy_row(toUpdate); return step; } |