[ovs-dev] [PATCH v2] ovsdb-cs: Fix use-after-free for the request id.

[Dumitru Ceara]

On 2/23/21 7:00 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 <i.maximets at ovn.org>
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara <dceara at redhat.com>