[ovs-git] [openvswitch/ovs] ac09cb: ovsdb-cs: Fix use-after-free for the request id.

Frode Nordahl noreply at github.com
Mon Mar 1 20:17:23 UTC 2021


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: ac09cbfcb70ac6f443f039d5934448bd80f74493
      https://github.com/openvswitch/ovs/commit/ac09cbfcb70ac6f443f039d5934448bd80f74493
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M lib/ovsdb-cs.c

  Log Message:
  -----------
  ovsdb-cs: Fix use-after-free for the request id.

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.")
Acked-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 14b2b0aad7ae9254bad8b8c2cc9d5386065ab42f
      https://github.com/openvswitch/ovs/commit/14b2b0aad7ae9254bad8b8c2cc9d5386065ab42f
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M ovsdb/raft.c

  Log Message:
  -----------
  raft: Reintroduce jsonrpc inactivity probes.

It's not enough to just have heartbeats.

RAFT heartbeats are unidirectional, i.e. leader sends them to followers
but not the other way around.  Missing heartbeats provokes followers to
start election, but if leader will not receive any replies it will not
do anything while there is a quorum, i.e. there are enough other
servers to make decisions.

This leads to situation that while TCP connection is established,
leader will continue to blindly send messages to it.  In our case this
leads to growing send backlog.  Connection will be terminated
eventually due to excessive send backlog, but this this might take a
lot of time and wasted process memory.  At the same time 'candidate'
will continue to send vote requests to the dead connection on its
side.

To fix that we need to reintroduce inactivity probes that will drop
connection if there was no incoming traffic for a long time and remote
server doesn't reply to the "echo" request.  Probe interval might be
chosen based on an election timeout to avoid issues described in commit
db5a066c17bd.

Reported-by: Carlos Goncalves <cgoncalves at redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1929690
Fixes: db5a066c17bd ("raft: Disable RAFT jsonrpc inactivity probe.")
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 4c1d9ef14af3da3b904c1ceb4a6823bb3c5fd3e3
      https://github.com/openvswitch/ovs/commit/4c1d9ef14af3da3b904c1ceb4a6823bb3c5fd3e3
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M ovsdb/raft.c

  Log Message:
  -----------
  raft: Report disconnected in cluster/status if candidate retries election.

If election times out for a server in 'candidate' role it sets
'candidate_retrying' flag that notifies that storage is disconnected
and client should re-connect.  However, cluster/status command
reports 'Status: cluster member' and that is misleading.
Reporting "disconnected from the cluster (election timeout)" instead.

Reported-by: Carlos Goncalves <cgoncalves at redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1929690
Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.")
Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 39b937f064347884614002c9bdac79382a144fca
      https://github.com/openvswitch/ovs/commit/39b937f064347884614002c9bdac79382a144fca
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M ovsdb/raft.c

  Log Message:
  -----------
  raft: Add 'stop-raft-rpc' failure test command.

This command will stop sending and receiving any RAFT-related
traffic or accepting new connections.  Useful to simulate
network problems between cluster members.

There is no unit test that uses it yet, but it's convenient for
manual testing.

Acked-by: Han Zhou <hzhou at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: f013e6f2d608719f190c7dadb6849bed07a9e6eb
      https://github.com/openvswitch/ovs/commit/f013e6f2d608719f190c7dadb6849bed07a9e6eb
  Author: William Tu <u9012063 at gmail.com>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M Documentation/topics/dpdk/qos.rst
    M vswitchd/vswitch.xml

  Log Message:
  -----------
  Documentation: Fix DPDK qos example.

Fix the example use case based on the decription.
EIR and CIR are measured in bytes/sec and considered 64-byte
IP packets size withtout 14-byte Ethernet header.
So fix the 1000pps example by: (64 - 14) * 1000 = 50,000
If the frame includes 4-byte FCS header, then it's
(64 - 14 - 4) * 1000 = 46,000

Fixes: e61bdffc2a98 ("netdev-dpdk: Add new DPDK RFC 4115 egress policer")
Signed-off-by: William Tu <u9012063 at gmail.com>
Acked-by: Eelco Chaudron <echaudro at redhat.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 2ad201659cedbb1134a9d27af132e491173c7e40
      https://github.com/openvswitch/ovs/commit/2ad201659cedbb1134a9d27af132e491173c7e40
  Author: Frode Nordahl <frode.nordahl at canonical.com>
  Date:   2021-03-01 (Mon, 01 Mar 2021)

  Changed paths:
    M NEWS
    M utilities/ovs-ctl.in

  Log Message:
  -----------
  ovs-ctl: Allow recording hostname separately.

ovs-ctl determines the system FQDN or hostname and records it in
the `external-ids:hostname` field of the `Open-vSwitch` table on
system startup if it is not already set.

This value may be consumed by downstream software and having it
unset or set to a incorrect value could lead to erratic behavior
of a system.

When a system is configured to use an Open vSwitch controlled
datapath as its only network connection, the current ordering of
events would always record a unreliable hostname.

To tackle this problem this patch adds an optional argument that
allows starting Open vSwitch without recording the hostname in
the database as well as a new ctl command to record the hostname
separately. This command can be called by the system startup
scripts when the system is ready to collect and record this
information.

Reported-At: https://bugs.launchpad.net/bugs/1915829
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/47e73f7f0058...2ad201659ced


More information about the git mailing list