On 2/23/21 2:55 PM, Ilya Maximets wrote:
> ovsdb_cs_send_transaction() returns the pointer to the same
> 'request_id' object that is used internally. This leads to
> situation where transaction in idl and CS module has the
> same 'request_id' object. However, CS module is able to
> destroy this transaction id at any time, e.g. if connection
> state chnaged, but idl transaction might be still around at
> this moment and application might still use it.
>
> Found by running 'make check-ovsdb-cluster' with AddressSanitizer:
>
> ==79922==ERROR: AddressSanitizer: heap-use-after-free on address
> 0x604000167a98 at pc 0x000000626acf bp 0x7ffcdb38a4c0 sp 0x7ffcdb38a4b8
> READ of size 8 at 0x604000167a98 thread T0
> #0 0x626ace in json_destroy lib/json.c:354:18
> #1 0x56d1ab in ovsdb_idl_txn_destroy lib/ovsdb-idl.c:2528:5
> #2 0x53a908 in do_vsctl utilities/ovs-vsctl.c:3008:5
> #3 0x539251 in main utilities/ovs-vsctl.c:203:17
> #4 0x7f7f7e376081 in __libc_start_main (/lib64/libc.so.6+0x27081)
> #5 0x461fed in _start (utilities/ovs-vsctl+0x461fed)
>
> 0x604000167a98 is located 8 bytes inside of 40-byte
> region [0x604000167a90,0x604000167ab8)
> freed by thread T0 here:
> #0 0x503ac7 in free (utilities/ovs-vsctl+0x503ac7)
> #1 0x626aae in json_destroy lib/json.c:378:9
> #2 0x6adfa2 in ovsdb_cs_run lib/ovsdb-cs.c:625:13
> #3 0x567731 in ovsdb_idl_run lib/ovsdb-idl.c:394:5
> #4 0x56fed1 in ovsdb_idl_txn_commit_block lib/ovsdb-idl.c:3187:9
> #5 0x53a4df in do_vsctl utilities/ovs-vsctl.c:2898:14
> #6 0x539251 in main utilities/ovs-vsctl.c:203:17
> #7 0x7f7f7e376081 in __libc_start_main
>
> previously allocated by thread T0 here:
> #0 0x503dcf in malloc (utilities/ovs-vsctl+0x503dcf)
> #1 0x594656 in xmalloc lib/util.c:138:15
> #2 0x626431 in json_create lib/json.c:1451:25
> #3 0x626972 in json_integer_create lib/json.c:263:25
> #4 0x62da0f in jsonrpc_create_id lib/jsonrpc.c:563:12
> #5 0x62d9a8 in jsonrpc_create_request lib/jsonrpc.c:570:23
> #6 0x6af3a6 in ovsdb_cs_send_transaction lib/ovsdb-cs.c:1357:35
> #7 0x56e3d5 in ovsdb_idl_txn_commit lib/ovsdb-idl.c:3147:27
> #8 0x56fea9 in ovsdb_idl_txn_commit_block lib/ovsdb-idl.c:3186:22
> #9 0x53a4df in do_vsctl utilities/ovs-vsctl.c:2898:14
> #10 0x539251 in main utilities/ovs-vsctl.c:203:17
> #11 0x7f7f7e376081 in __libc_start_main
>
> Fixes: 1c337c43ac1c ("ovsdb-idl: Break into two layers.")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
> lib/ovsdb-cs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
> index e0934772e..ded7cc455 100644
> --- a/lib/ovsdb-cs.c
> +++ b/lib/ovsdb-cs.c
> @@ -1367,7 +1367,7 @@ ovsdb_cs_send_transaction(struct ovsdb_cs *cs, struct
> json *operations)
> sizeof *cs->txns);
> }
> cs->txns[cs->n_txns++] = request_id;
> - return request_id;
> + return json_clone(request_id);
> }
>
> /* Makes 'cs' drop its record of transaction 'request_id'. If a reply
> arrives
>
Sorry, this needs more work. Apparently, this request_id gets leaked somewhere
else with this change applied. Will track it down and send v2.
Bets regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
