Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation update #2

Merged
merged 2 commits into from
Jul 11, 2019
Merged

Documentation update #2

merged 2 commits into from
Jul 11, 2019

Conversation

ryzhyk
Copy link
Collaborator

@ryzhyk ryzhyk commented Jul 10, 2019

No description provided.

When using the DDlog record/replay feature, truncate the `replay.dat`
file before recording commands to it.
Documented recent changes to the ddlog infrastructue.
@justinpettit justinpettit merged commit b488de6 into ovn-org:ddlog-dev Jul 11, 2019
numansiddique referenced this pull request in numansiddique/ovn Jul 25, 2019
Running ovsdb-server with empty private-key and non-empty certificate
(or otherwise) causes crash:

 # ovsdb-tool create ./etc/openvswitch/conf.db ./vswitch.ovsschema
 # ovsdb-server --remote=punix:./db.sock \
                --remote=db:Open_vSwitch,Open_vSwitch,manager_options \
                --private-key=db:Open_vSwitch,SSL,private_key \
                --certificate=db:Open_vSwitch,SSL,certificate \
                --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert

 # ovs-vsctl --no-wait init
 # ovs-vsctl --no-wait set-ssl pkey.key cert.cert ca.cert
 # ovs-vsctl --no-wait set SSL . private_key='""'
 # ovs-vsctl --no-wait set SSL . certificate='cert.new'

 ==25513==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
 ==25513==The signal is caused by a READ memory access.
 ==25513==Hint: address points to the zero page.
    #0 0x7ff7582aa0a9 in __GI___strlen_sse2
    #1 0x7ff759bdde81  (/lib64/libasan.so.5+0xace81)
    #2 0x7ff759479932  (/lib64/libcrypto.so.1.1+0xb3932)
    ovn-org#3 0x7ff759473c5a in BIO_ctrl (/lib64/libcrypto.so.1.1+0xadc5a)
    ovn-org#4 0x7ff7598decc1 in SSL_CTX_use_certificate_file (/lib64/libssl.so.1.1+0x40cc1)
    ovn-org#5 0x4dbaa7 in stream_ssl_set_certificate_file__ lib/stream-ssl.c:1170
    ovn-org#6 0x4dca2e in stream_ssl_set_key_and_cert lib/stream-ssl.c:1216
    ovn-org#7 0x4146b2 in reconfigure_ssl ovsdb/ovsdb-server.c:1254
    ovn-org#8 0x409c83 in main ovsdb/ovsdb-server.c:368
    ovn-org#9 0x7ff758233812 in __libc_start_main
    ovn-org#10 0x40f6bd in _start (ovsdb-server+0x40f6bd)

 AddressSanitizer can not provide additional info.
 SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x9a0a9) in __GI___strlen_sse2
 ==25513==ABORTING

Another way to reproduce is to use non-initialized DB entry for
private-key and a file for certificate in ovsdb-server cmdline.

The root cause is that stream_ssl_set_key_and_cert() triggers
configuration for both key and cert if any of them is valid, keeping
it possible for one of them to be NULL.

Fixes: 6f1e91b ("stream-ssl: Make changing keys and certificate at runtime reliable.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Ben Pfaff <blp@ovn.org>
numansiddique referenced this pull request in numansiddique/ovn Jul 25, 2019
The following, run inside the OVS sandbox, caused OVS to abort when
Address Sanitizer was used:

    ovs-vsctl add-br br-int
    ovs-ofctl add-flow br-int "table=0,cookie=0x1234,priority=10000,icmp,actions=drop"
    ovs-ofctl --strict del-flows br-int "table=0,cookie=0x1234/-1,priority=10000"

Sample report from Address Sanitizer:

==3029==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000043260 at pc 0x7f6b09c2459b bp 0x7ffcb67e7540 sp 0x7ffcb67e6cf0
READ of size 40 at 0x603000043260 thread T0
    #0 0x7f6b09c2459a  (/lib/x86_64-linux-gnu/libasan.so.5+0xb859a)
    #1 0x565110a748a5 in minimask_equal ../lib/flow.c:3510
    #2 0x565110a9ea41 in minimatch_equal ../lib/match.c:1821
    ovn-org#3 0x56511091e864 in collect_rules_strict ../ofproto/ofproto.c:4516
    ovn-org#4 0x56511093d526 in delete_flow_start_strict ../ofproto/ofproto.c:5959
    ovn-org#5 0x56511093d526 in ofproto_flow_mod_start ../ofproto/ofproto.c:7949
    ovn-org#6 0x56511093d77b in handle_flow_mod__ ../ofproto/ofproto.c:6122
    ovn-org#7 0x56511093db71 in handle_flow_mod ../ofproto/ofproto.c:6099
    ovn-org#8 0x5651109407f6 in handle_single_part_openflow ../ofproto/ofproto.c:8406
    ovn-org#9 0x5651109407f6 in handle_openflow ../ofproto/ofproto.c:8587
    ovn-org#10 0x5651109e40da in ofconn_run ../ofproto/connmgr.c:1318
    ovn-org#11 0x5651109e40da in connmgr_run ../ofproto/connmgr.c:355
    ovn-org#12 0x56511092b129 in ofproto_run ../ofproto/ofproto.c:1826
    ovn-org#13 0x5651108f23cd in bridge_run__ ../vswitchd/bridge.c:2965
    ovn-org#14 0x565110904887 in bridge_run ../vswitchd/bridge.c:3023
    ovn-org#15 0x5651108e659c in main ../vswitchd/ovs-vswitchd.c:127
    ovn-org#16 0x7f6b093b709a in __libc_start_main ../csu/libc-start.c:308
    ovn-org#17 0x5651108e9009 in _start (/home/blp/nicira/ovs/_build/vswitchd/ovs-vswitchd+0x11d009)

This fixes the problem, which although largely theoretical could crop up
with odd implementations of memcmp(), perhaps ones optimized in various
"clever" ways.  All in all, it seems best to avoid the theoretical problem.

Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
numansiddique referenced this pull request in numansiddique/ovn Sep 11, 2019
Problem Description:
The ovs-vswitchd is crashing while invoking flow-mod with upsupported
action(Tested with ovs2.10.1)

Steps to recreate:
Step 1) Create a flow
ovs-ofctl add-flow switch1
priority=228,dl_type=0x0800,dl_vlan="600",in_port=25,actions=output:ALL
This step is successful.

Step 2) Invoke flow-mod with incorrect contents.
ovs-ofctl mod-flows switch1
priority=228,dl_type=0x0800,dl_vlan="600",in_port=25,actions=output:ALL,mod_vlan_vid:50,mod_vlan_pcp=6,mod_nw_tos=16

In the above example, the ofproto provider I have, will return error for
rule_construct as set_fields come after Output.

However the OVS is ignoring the error (The return value of add_flow_init
is ignored in modify_flow_init_strict) and eventually the ovs-vswitched
crashes.

Crash backtrace:
-----------------------

Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault.

 0x00007f6a06e785fb in modify_flows_start__ (
   ofproto=ofproto@entry=0x55b289cecc28, ofm=ofm@entry=0x7ffdf7d57b70)
     at ofproto/ofproto.c:5402
 5402    in ofproto/ofproto.c
 (gdb) bt
 #0  0x00007f6a06e785fb in modify_flows_start__ (
   ofproto=ofproto@entry=0x55b289cecc28, ofm=ofm@entry=0x7ffdf7d57b70)
    at ofproto/ofproto.c:5402
 #1  0x00007f6a06e790db in modify_flows_start_loose (ofm=0x7ffdf7d57b70,
    ofproto=0x55b289cecc28) at ofproto/ofproto.c:5443
 #2  ofproto_flow_mod_start (ofproto=ofproto@entry=0x55b289cecc28,
     ofm=ofm@entry=0x7ffdf7d57b70) at ofproto/ofproto.c:7672
 ovn-org#3  0x00007f6a06e79164 in handle_flow_mod__ (
    ofproto=ofproto@entry=0x55b289cecc28, fm=fm@entry=0x7ffdf7d57d20,
    req=req@entry=0x7ffdf7d57cd0) at ofproto/ofproto.c:5858
 ovn-org#4  0x00007f6a06e792c2 in handle_flow_mod (ofconn=ofconn@entry
 =0x55b289d528c0,
oh=oh@entry=0x55b289d5a410) at ofproto/ofproto.c:5835
 ovn-org#5  0x00007f6a06e7a173 in handle_openflow__ (msg=0x55b289d351d0,
    ofconn=0x55b289d528c0) at ofproto/ofproto.c:8127
 ovn-org#6  handle_openflow (ofconn=0x55b289d528c0, ofp_msg=0x55b289d351d0)
  at ofproto/ofproto.c:8296
 ovn-org#7  0x00007f6a06e6a013 in ofconn_run (
  handle_openflow=0x7f6a06e796f0 <handle_openflow>,
 ofconn=0x55b289d528c0)
 at ofproto/connmgr.c:1446
 ovn-org#8  connmgr_run (mgr=0x55b289d14fe0,
 handle_openflow=handle_openflow@entry=0x7f6a06e796f0
handle_openflow>)
at ofproto/connmgr.c:365

With this fix, OVS does not crash.

Signed-off-by: Parameswaran Krishnamurthy <parkrish@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
numansiddique referenced this pull request in numansiddique/ovn Sep 11, 2019
Running ovsdb-server with empty private-key and non-empty certificate
(or otherwise) causes crash:

 # ovsdb-tool create ./etc/openvswitch/conf.db ./vswitch.ovsschema
 # ovsdb-server --remote=punix:./db.sock \
                --remote=db:Open_vSwitch,Open_vSwitch,manager_options \
                --private-key=db:Open_vSwitch,SSL,private_key \
                --certificate=db:Open_vSwitch,SSL,certificate \
                --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert

 # ovs-vsctl --no-wait init
 # ovs-vsctl --no-wait set-ssl pkey.key cert.cert ca.cert
 # ovs-vsctl --no-wait set SSL . private_key='""'
 # ovs-vsctl --no-wait set SSL . certificate='cert.new'

 ==25513==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
 ==25513==The signal is caused by a READ memory access.
 ==25513==Hint: address points to the zero page.
    #0 0x7ff7582aa0a9 in __GI___strlen_sse2
    #1 0x7ff759bdde81  (/lib64/libasan.so.5+0xace81)
    #2 0x7ff759479932  (/lib64/libcrypto.so.1.1+0xb3932)
    ovn-org#3 0x7ff759473c5a in BIO_ctrl (/lib64/libcrypto.so.1.1+0xadc5a)
    ovn-org#4 0x7ff7598decc1 in SSL_CTX_use_certificate_file (/lib64/libssl.so.1.1+0x40cc1)
    ovn-org#5 0x4dbaa7 in stream_ssl_set_certificate_file__ lib/stream-ssl.c:1170
    ovn-org#6 0x4dca2e in stream_ssl_set_key_and_cert lib/stream-ssl.c:1216
    ovn-org#7 0x4146b2 in reconfigure_ssl ovsdb/ovsdb-server.c:1254
    ovn-org#8 0x409c83 in main ovsdb/ovsdb-server.c:368
    ovn-org#9 0x7ff758233812 in __libc_start_main
    ovn-org#10 0x40f6bd in _start (ovsdb-server+0x40f6bd)

 AddressSanitizer can not provide additional info.
 SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x9a0a9) in __GI___strlen_sse2
 ==25513==ABORTING

Another way to reproduce is to use non-initialized DB entry for
private-key and a file for certificate in ovsdb-server cmdline.

The root cause is that stream_ssl_set_key_and_cert() triggers
configuration for both key and cert if any of them is valid, keeping
it possible for one of them to be NULL.

Fixes: 6f1e91b ("stream-ssl: Make changing keys and certificate at runtime reliable.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Ben Pfaff <blp@ovn.org>
numansiddique referenced this pull request in numansiddique/ovn Sep 11, 2019
The following, run inside the OVS sandbox, caused OVS to abort when
Address Sanitizer was used:

    ovs-vsctl add-br br-int
    ovs-ofctl add-flow br-int "table=0,cookie=0x1234,priority=10000,icmp,actions=drop"
    ovs-ofctl --strict del-flows br-int "table=0,cookie=0x1234/-1,priority=10000"

Sample report from Address Sanitizer:

==3029==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000043260 at pc 0x7f6b09c2459b bp 0x7ffcb67e7540 sp 0x7ffcb67e6cf0
READ of size 40 at 0x603000043260 thread T0
    #0 0x7f6b09c2459a  (/lib/x86_64-linux-gnu/libasan.so.5+0xb859a)
    #1 0x565110a748a5 in minimask_equal ../lib/flow.c:3510
    #2 0x565110a9ea41 in minimatch_equal ../lib/match.c:1821
    ovn-org#3 0x56511091e864 in collect_rules_strict ../ofproto/ofproto.c:4516
    ovn-org#4 0x56511093d526 in delete_flow_start_strict ../ofproto/ofproto.c:5959
    ovn-org#5 0x56511093d526 in ofproto_flow_mod_start ../ofproto/ofproto.c:7949
    ovn-org#6 0x56511093d77b in handle_flow_mod__ ../ofproto/ofproto.c:6122
    ovn-org#7 0x56511093db71 in handle_flow_mod ../ofproto/ofproto.c:6099
    ovn-org#8 0x5651109407f6 in handle_single_part_openflow ../ofproto/ofproto.c:8406
    ovn-org#9 0x5651109407f6 in handle_openflow ../ofproto/ofproto.c:8587
    ovn-org#10 0x5651109e40da in ofconn_run ../ofproto/connmgr.c:1318
    ovn-org#11 0x5651109e40da in connmgr_run ../ofproto/connmgr.c:355
    ovn-org#12 0x56511092b129 in ofproto_run ../ofproto/ofproto.c:1826
    ovn-org#13 0x5651108f23cd in bridge_run__ ../vswitchd/bridge.c:2965
    ovn-org#14 0x565110904887 in bridge_run ../vswitchd/bridge.c:3023
    ovn-org#15 0x5651108e659c in main ../vswitchd/ovs-vswitchd.c:127
    ovn-org#16 0x7f6b093b709a in __libc_start_main ../csu/libc-start.c:308
    ovn-org#17 0x5651108e9009 in _start (/home/blp/nicira/ovs/_build/vswitchd/ovs-vswitchd+0x11d009)

This fixes the problem, which although largely theoretical could crop up
with odd implementations of memcmp(), perhaps ones optimized in various
"clever" ways.  All in all, it seems best to avoid the theoretical problem.

Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
dceara pushed a commit to dceara/ovn that referenced this pull request Jun 18, 2020
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Oct 14, 2020
The 'nexthop' that passed to ic_route_hash() is not fully initialized in
get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' which
contains a union to share space for ipv4 and ipv6 address.  If only ipv4
initialized where is a plenty of uninitialized space that goes to
hash_bytes(nexthop, sizeof *nexthop, basis).

Impact: there are two places where this function is called.

1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before
   calling get_nexthop_from_lport_addresses(), luckily.

2. In add_network_to_routes_ad(), we are unlucky.  When a directly connected
network of a router is found to be advertised, if the route already existed in
the global IC-SB, it may not be found due to the hash difference, and results
in the existing route being deleted and the same one recreated, unnecessarily.

This patch fixes the problem by initializing the struct to zero before setting
the fields.

From Ilya's report:
> Report from MemorySanitizer:
>
> ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9
>     ovn-org#1 0x671668 in mhash_add ovs/./lib/hash.h:78:12
>     ovn-org#2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16
>     ovn-org#3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5
>     ovn-org#4 0x51eea3 in route_run ic/ovn-ic.c:1424:21
>     ovn-org#5 0x51887b in main ic/ovn-ic.c:1674:17
>     ovn-org#6 0x7fd4ce7871a2 in __libc_start_main
>     ovn-org#7 0x49c90d in _start (ic/ovn-ic+0x49c90d)
>
>   Uninitialized value was created by an allocation of 'nexthop' in the
>   stack frame of function 'add_network_to_routes_ad'
>     #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
Fixes: 57b347c ("ovn-ic: Route advertisement.")
Signed-off-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
hzhou8 added a commit that referenced this pull request Oct 21, 2020
The 'nexthop' that passed to ic_route_hash() is not fully initialized in
get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' which
contains a union to share space for ipv4 and ipv6 address.  If only ipv4
initialized where is a plenty of uninitialized space that goes to
hash_bytes(nexthop, sizeof *nexthop, basis).

Impact: there are two places where this function is called.

1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before
   calling get_nexthop_from_lport_addresses(), luckily.

2. In add_network_to_routes_ad(), we are unlucky.  When a directly connected
network of a router is found to be advertised, if the route already existed in
the global IC-SB, it may not be found due to the hash difference, and results
in the existing route being deleted and the same one recreated, unnecessarily.

This patch fixes the problem by initializing the struct to zero before setting
the fields.

From Ilya's report:
> Report from MemorySanitizer:
>
> ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9
>     #1 0x671668 in mhash_add ovs/./lib/hash.h:78:12
>     #2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16
>     #3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5
>     #4 0x51eea3 in route_run ic/ovn-ic.c:1424:21
>     #5 0x51887b in main ic/ovn-ic.c:1674:17
>     #6 0x7fd4ce7871a2 in __libc_start_main
>     #7 0x49c90d in _start (ic/ovn-ic+0x49c90d)
>
>   Uninitialized value was created by an allocation of 'nexthop' in the
>   stack frame of function 'add_network_to_routes_ad'
>     #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
Fixes: 57b347c ("ovn-ic: Route advertisement.")
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
hzhou8 added a commit that referenced this pull request Oct 21, 2020
The 'nexthop' that passed to ic_route_hash() is not fully initialized in
get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' which
contains a union to share space for ipv4 and ipv6 address.  If only ipv4
initialized where is a plenty of uninitialized space that goes to
hash_bytes(nexthop, sizeof *nexthop, basis).

Impact: there are two places where this function is called.

1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before
   calling get_nexthop_from_lport_addresses(), luckily.

2. In add_network_to_routes_ad(), we are unlucky.  When a directly connected
network of a router is found to be advertised, if the route already existed in
the global IC-SB, it may not be found due to the hash difference, and results
in the existing route being deleted and the same one recreated, unnecessarily.

This patch fixes the problem by initializing the struct to zero before setting
the fields.

From Ilya's report:
> Report from MemorySanitizer:
>
> ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9
>     #1 0x671668 in mhash_add ovs/./lib/hash.h:78:12
>     #2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16
>     #3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5
>     #4 0x51eea3 in route_run ic/ovn-ic.c:1424:21
>     #5 0x51887b in main ic/ovn-ic.c:1674:17
>     #6 0x7fd4ce7871a2 in __libc_start_main
>     #7 0x49c90d in _start (ic/ovn-ic+0x49c90d)
>
>   Uninitialized value was created by an allocation of 'nexthop' in the
>   stack frame of function 'add_network_to_routes_ad'
>     #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
Fixes: 57b347c ("ovn-ic: Route advertisement.")
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
hzhou8 added a commit that referenced this pull request Oct 21, 2020
The 'nexthop' that passed to ic_route_hash() is not fully initialized in
get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' which
contains a union to share space for ipv4 and ipv6 address.  If only ipv4
initialized where is a plenty of uninitialized space that goes to
hash_bytes(nexthop, sizeof *nexthop, basis).

Impact: there are two places where this function is called.

1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before
   calling get_nexthop_from_lport_addresses(), luckily.

2. In add_network_to_routes_ad(), we are unlucky.  When a directly connected
network of a router is found to be advertised, if the route already existed in
the global IC-SB, it may not be found due to the hash difference, and results
in the existing route being deleted and the same one recreated, unnecessarily.

This patch fixes the problem by initializing the struct to zero before setting
the fields.

From Ilya's report:
> Report from MemorySanitizer:
>
> ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9
>     #1 0x671668 in mhash_add ovs/./lib/hash.h:78:12
>     #2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16
>     #3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5
>     #4 0x51eea3 in route_run ic/ovn-ic.c:1424:21
>     #5 0x51887b in main ic/ovn-ic.c:1674:17
>     #6 0x7fd4ce7871a2 in __libc_start_main
>     #7 0x49c90d in _start (ic/ovn-ic+0x49c90d)
>
>   Uninitialized value was created by an allocation of 'nexthop' in the
>   stack frame of function 'add_network_to_routes_ad'
>     #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
Fixes: 57b347c ("ovn-ic: Route advertisement.")
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
hzhou8 added a commit that referenced this pull request Oct 21, 2020
The 'nexthop' that passed to ic_route_hash() is not fully initialized in
get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' which
contains a union to share space for ipv4 and ipv6 address.  If only ipv4
initialized where is a plenty of uninitialized space that goes to
hash_bytes(nexthop, sizeof *nexthop, basis).

Impact: there are two places where this function is called.

1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before
   calling get_nexthop_from_lport_addresses(), luckily.

2. In add_network_to_routes_ad(), we are unlucky.  When a directly connected
network of a router is found to be advertised, if the route already existed in
the global IC-SB, it may not be found due to the hash difference, and results
in the existing route being deleted and the same one recreated, unnecessarily.

This patch fixes the problem by initializing the struct to zero before setting
the fields.

From Ilya's report:
> Report from MemorySanitizer:
>
> ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9
>     #1 0x671668 in mhash_add ovs/./lib/hash.h:78:12
>     #2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16
>     #3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5
>     #4 0x51eea3 in route_run ic/ovn-ic.c:1424:21
>     #5 0x51887b in main ic/ovn-ic.c:1674:17
>     #6 0x7fd4ce7871a2 in __libc_start_main
>     #7 0x49c90d in _start (ic/ovn-ic+0x49c90d)
>
>   Uninitialized value was created by an allocation of 'nexthop' in the
>   stack frame of function 'add_network_to_routes_ad'
>     #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
Fixes: 57b347c ("ovn-ic: Route advertisement.")
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 19, 2020
Result of 'normalize_v46_prefix()' should be freed and all dynamic
strings should be destroyed.

  Direct leak of 156 byte(s) in 13 object(s) allocated from:
    #0 0x53700f in malloc (northd/ovn-northd+0x53700f)
    ovn-org#1 0x71b146 in xmalloc ovs/lib/util.c:138:15
    ovn-org#2 0x71b39a in xvasprintf ovs/lib/util.c:202:9
    ovn-org#3 0x71b753 in xasprintf ovs/lib/util.c:343:9
    ovn-org#4 0x59bddc in add_ecmp_symmetric_reply_flows northd/ovn-northd.c:7985:18
    ovn-org#5 0x59aab5 in build_ecmp_route_flow northd/ovn-northd.c:8102:13
    ovn-org#6 0x592260 in build_static_route_flows_for_lrouter ovn-northd.c:10243:13
    ovn-org#7 0x589267 in build_lswitch_and_lrouter_iterate_by_od ovn-northd.c:11241:5
    ovn-org#8 0x5888ca in build_lswitch_and_lrouter_flows northd/ovn-northd.c:11313:9
    ovn-org#9 0x5730a0 in build_lflows northd/ovn-northd.c:11401:5
    ovn-org#10 0x56f992 in ovnnb_db_run northd/ovn-northd.c:12339:5
    ovn-org#11 0x56e8a7 in ovn_db_run northd/ovn-northd.c:12932:5
    ovn-org#12 0x56d6af in main northd/ovn-northd.c:13338:17
    ovn-org#13 0x7f5371d041a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Fixes: 4fdca65 ("Add ECMP symmetric replies.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
'child_port_list' is an array of pointers that should be freed too.

  Direct leak of 30 byte(s) in 6 object(s) allocated from:
    #0 0x501fff in malloc (/tests/ovstest+0x501fff)
    ovn-org#1 0x6227e6 in xmalloc /lib/util.c:138:15
    ovn-org#2 0x6228b8 in xmemdup0 /lib/util.c:168:15
    ovn-org#3 0x8183d6 in parse_fwd_group_action /lib/actions.c:3374:30
    ovn-org#4 0x814b6e in parse_action /lib/actions.c:3610:9
    ovn-org#5 0x8139ef in parse_actions /lib/actions.c:3637:14
    ovn-org#6 0x8136a3 in ovnacts_parse /lib/actions.c:3672:9
    ovn-org#7 0x813c80 in ovnacts_parse_string /lib/actions.c:3699:5
    ovn-org#8 0x53a979 in test_parse_actions /tests/test-ovn.c:1372:21
    ovn-org#9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    ovn-org#10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    ovn-org#11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    ovn-org#12 0x537359 in main /tests/ovstest.c:133:9
    ovn-org#13 0x7f06978f21a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Manoj Sharma <manoj.sharma@nutanix.com>
Fixes: edb2400 ("Forwarding group to load balance l2 traffic with liveness detection")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
'dsts' should be freed in case of any error.

  Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x502378 in realloc (/tests/ovstest+0x502378)
    ovn-org#1 0x622826 in xrealloc /lib/util.c:149:9
    ovn-org#2 0x8194f4 in parse_select_action /lib/actions.c:1185:20
    ovn-org#3 0x814f49 in parse_set_action /lib/actions.c:3499:13
    ovn-org#4 0x814341 in parse_action /lib/actions.c:3554:9
    ovn-org#5 0x8139ef in parse_actions /lib/actions.c:3643:14
    ovn-org#6 0x8136a3 in ovnacts_parse /lib/actions.c:3678:9
    ovn-org#7 0x813c80 in ovnacts_parse_string /lib/actions.c:3705:5
    ovn-org#8 0x53a4e8 in test_parse_actions /tests/test-ovn.c:1321:17
    ovn-org#9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    ovn-org#10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    ovn-org#11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    ovn-org#12 0x537359 in main /tests/ovstest.c:133:9
    ovn-org#13 0x7f9ce05ba1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Han Zhou <hzhou@ovn.org>
Fixes: 85b3544 ("ovn-controller: A new action "select".")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
'parse_ofp_meter_mod_str' allocates space for meter.bands that
should be freed.

  Direct leak of 448 byte(s) in 7 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    ovn-org#1 0x7523a6 in xmalloc /lib/util.c:138:15
    ovn-org#2 0x6fd079 in ofpbuf_init /lib/ofpbuf.c:123:26
    ovn-org#3 0x6cba27 in parse_ofp_meter_mod_str /lib/ofp-meter.c:779:5
    ovn-org#4 0x5705b8 in add_meter_string /controller/ofctrl.c:1674:19
    ovn-org#5 0x56f736 in ofctrl_put /controller/ofctrl.c:2105:13
    ovn-org#6 0x59aebb in main /controller/ovn-controller.c:2627:25
    ovn-org#7 0x7f07873251a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Guoshuai Li <ligs@dtdream.com>
Fixes: c25094b ("ovn: OVN Support QoS meter")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
'smap_clear()' doesn't free allocated memory, but 'smap_clone()'
re-initializes hash map clearing internal pointers and leaking
this memory.  'smap_destroy()' should be used instead.

Also, all the records and array of datapaths should be freed on
destruction of a cache entry.

  Direct leak of 16 byte(s) in 2 object(s) allocated from:
    #0 0x5211c7 in calloc (/controller/ovn-controller+0x5211c7)
    ovn-org#1 0x752364 in xcalloc /lib/util.c:121:31
    ovn-org#2 0x576e76 in sync_dns_cache /controller/pinctrl.c:2517:25
    ovn-org#3 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    ovn-org#4 0x59b06c in main /controller/ovn-controller.c:2642:25
    ovn-org#5 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

  Indirect leak of 26 byte(s) in 2 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    ovn-org#1 0x7523d6 in xmalloc /lib/util.c:138:15
    ovn-org#2 0x7524a8 in xmemdup0 /lib/util.c:168:15
    ovn-org#3 0x73d8fc in smap_clone /lib/smap.c:314:45
    ovn-org#4 0x576e2f in sync_dns_cache /controller/pinctrl.c:2513:13
    ovn-org#5 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    ovn-org#6 0x59b06c in main /controller/ovn-controller.c:2642:25
    ovn-org#7 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Fixes: 6b72068 ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
shash contains pointers to the data that should be freed.

  Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    ovn-org#1 0x752436 in xmalloc /lib/util.c:138:15
    ovn-org#2 0x5a2f0b in add_pending_ct_zone_entry /controller/ovn-controller.c:548:45
    ovn-org#3 0x5a2d1d in update_ct_zones /controller/ovn-controller.c:668:9
    ovn-org#4 0x59d8c6 in en_ct_zones_run /controller/ovn-controller.c:1495:5
    ovn-org#5 0x5dade4 in engine_run /lib/inc-proc-eng.c:377:9
    ovn-org#6 0x59adf4 in main /controller/ovn-controller.c
    ovn-org#7 0x7f0799ef41a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: xu rong <xu.rong@zte.com.cn>
Fixes: 252e164 ("ovn-controller: pending_ct_zones should be destroyed")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Nov 20, 2020
When a port binding of type "l3gateway" is claimed its remote peer
port_binding is also stored in local_datapath.peer_ports[].remote.

If the remote peer port_binding is deleted first (i.e., before the local
"l3gateway" one) then we need to remove the complete
local_datapath.peer_ports[] entry in order to avoid ending up using
dangling pointers to already freed port bindings.

Also, properly reset local_datapath->has_local_l3gateway in
remove_pb_from_local_datapath().

Ilya reported this issue found by AddressSanitizer during his testing:

==1816017==ERROR: AddressSanitizer: heap-use-after-free on address 0x6140000cb170 at pc 0x0000005ab574 bp 0x7fff68925a30 sp 0x7fff68925a28
READ of size 8 at 0x6140000cb170 thread T0
    #0 0x5ab573 in put_replace_chassis_mac_flows git/ovn/controller/physical.c:550:9
    ovn-org#1 0x5a65eb in consider_port_binding git/ovn/controller/physical.c:1168:13
    ovn-org#2 0x5a8764 in physical_run git/ovn/controller/physical.c:1607:9
    ovn-org#3 0x5a0064 in flow_output_physical_flow_changes_handler git/ovn/controller/ovn-controller.c:2127:9
    ovn-org#4 0x5db423 in engine_compute git/ovn/lib/inc-proc-eng.c:306:18
    ovn-org#5 0x5dae1f in engine_run_node git/ovn/lib/inc-proc-eng.c:352:14
    ovn-org#6 0x5dac74 in engine_run git/ovn/lib/inc-proc-eng.c:377:9
    ovn-org#7 0x59ad64 in main git/ovn/controller/ovn-controller.c
    ovn-org#8 0x7f39fa6491a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    ovn-org#9 0x480b2d in _start (git/ovn/controller/ovn-controller+0x480b2d)

0x6140000cb170 is located 304 bytes inside of 408-byte region [0x6140000cb040,0x6140000cb1d8)
freed by thread T0 here:
    #0 0x520d07 in free (git/ovn/controller/ovn-controller+0x520d07)
    ovn-org#1 0x712de7 in ovsdb_idl_db_track_clear git/ovs/lib/ovsdb-idl.c:1984:21
    ovn-org#2 0x59b5cd in main git/ovn/controller/ovn-controller.c:2762:9
    ovn-org#3 0x7f39fa6491a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Fixes: 354bdba ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'child_port_list' is an array of pointers that should be freed too.

  Direct leak of 30 byte(s) in 6 object(s) allocated from:
    #0 0x501fff in malloc (/tests/ovstest+0x501fff)
    #1 0x6227e6 in xmalloc /lib/util.c:138:15
    #2 0x6228b8 in xmemdup0 /lib/util.c:168:15
    #3 0x8183d6 in parse_fwd_group_action /lib/actions.c:3374:30
    #4 0x814b6e in parse_action /lib/actions.c:3610:9
    #5 0x8139ef in parse_actions /lib/actions.c:3637:14
    #6 0x8136a3 in ovnacts_parse /lib/actions.c:3672:9
    #7 0x813c80 in ovnacts_parse_string /lib/actions.c:3699:5
    #8 0x53a979 in test_parse_actions /tests/test-ovn.c:1372:21
    #9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    #11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #12 0x537359 in main /tests/ovstest.c:133:9
    #13 0x7f06978f21a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Manoj Sharma <manoj.sharma@nutanix.com>
Fixes: edb2400 ("Forwarding group to load balance l2 traffic with liveness detection")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'dsts' should be freed in case of any error.

  Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x502378 in realloc (/tests/ovstest+0x502378)
    #1 0x622826 in xrealloc /lib/util.c:149:9
    #2 0x8194f4 in parse_select_action /lib/actions.c:1185:20
    #3 0x814f49 in parse_set_action /lib/actions.c:3499:13
    #4 0x814341 in parse_action /lib/actions.c:3554:9
    #5 0x8139ef in parse_actions /lib/actions.c:3643:14
    #6 0x8136a3 in ovnacts_parse /lib/actions.c:3678:9
    #7 0x813c80 in ovnacts_parse_string /lib/actions.c:3705:5
    #8 0x53a4e8 in test_parse_actions /tests/test-ovn.c:1321:17
    #9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    #11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #12 0x537359 in main /tests/ovstest.c:133:9
    #13 0x7f9ce05ba1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Han Zhou <hzhou@ovn.org>
Fixes: 85b3544 ("ovn-controller: A new action "select".")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'parse_ofp_meter_mod_str' allocates space for meter.bands that
should be freed.

  Direct leak of 448 byte(s) in 7 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    #1 0x7523a6 in xmalloc /lib/util.c:138:15
    #2 0x6fd079 in ofpbuf_init /lib/ofpbuf.c:123:26
    #3 0x6cba27 in parse_ofp_meter_mod_str /lib/ofp-meter.c:779:5
    #4 0x5705b8 in add_meter_string /controller/ofctrl.c:1674:19
    #5 0x56f736 in ofctrl_put /controller/ofctrl.c:2105:13
    #6 0x59aebb in main /controller/ovn-controller.c:2627:25
    #7 0x7f07873251a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Guoshuai Li <ligs@dtdream.com>
Fixes: c25094b ("ovn: OVN Support QoS meter")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'smap_clear()' doesn't free allocated memory, but 'smap_clone()'
re-initializes hash map clearing internal pointers and leaking
this memory.  'smap_destroy()' should be used instead.

Also, all the records and array of datapaths should be freed on
destruction of a cache entry.

  Direct leak of 16 byte(s) in 2 object(s) allocated from:
    #0 0x5211c7 in calloc (/controller/ovn-controller+0x5211c7)
    #1 0x752364 in xcalloc /lib/util.c:121:31
    #2 0x576e76 in sync_dns_cache /controller/pinctrl.c:2517:25
    #3 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    #4 0x59b06c in main /controller/ovn-controller.c:2642:25
    #5 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

  Indirect leak of 26 byte(s) in 2 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    #1 0x7523d6 in xmalloc /lib/util.c:138:15
    #2 0x7524a8 in xmemdup0 /lib/util.c:168:15
    #3 0x73d8fc in smap_clone /lib/smap.c:314:45
    #4 0x576e2f in sync_dns_cache /controller/pinctrl.c:2513:13
    #5 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    #6 0x59b06c in main /controller/ovn-controller.c:2642:25
    #7 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Fixes: 6b72068 ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
shash contains pointers to the data that should be freed.

  Direct leak of 32 byte(s) in 2 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    #1 0x752436 in xmalloc /lib/util.c:138:15
    #2 0x5a2f0b in add_pending_ct_zone_entry /controller/ovn-controller.c:548:45
    #3 0x5a2d1d in update_ct_zones /controller/ovn-controller.c:668:9
    #4 0x59d8c6 in en_ct_zones_run /controller/ovn-controller.c:1495:5
    #5 0x5dade4 in engine_run /lib/inc-proc-eng.c:377:9
    #6 0x59adf4 in main /controller/ovn-controller.c
    #7 0x7f0799ef41a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: xu rong <xu.rong@zte.com.cn>
Fixes: 252e164 ("ovn-controller: pending_ct_zones should be destroyed")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'child_port_list' is an array of pointers that should be freed too.

  Direct leak of 30 byte(s) in 6 object(s) allocated from:
    #0 0x501fff in malloc (/tests/ovstest+0x501fff)
    #1 0x6227e6 in xmalloc /lib/util.c:138:15
    #2 0x6228b8 in xmemdup0 /lib/util.c:168:15
    #3 0x8183d6 in parse_fwd_group_action /lib/actions.c:3374:30
    #4 0x814b6e in parse_action /lib/actions.c:3610:9
    #5 0x8139ef in parse_actions /lib/actions.c:3637:14
    #6 0x8136a3 in ovnacts_parse /lib/actions.c:3672:9
    #7 0x813c80 in ovnacts_parse_string /lib/actions.c:3699:5
    #8 0x53a979 in test_parse_actions /tests/test-ovn.c:1372:21
    #9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    #11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #12 0x537359 in main /tests/ovstest.c:133:9
    #13 0x7f06978f21a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Manoj Sharma <manoj.sharma@nutanix.com>
Fixes: edb2400 ("Forwarding group to load balance l2 traffic with liveness detection")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'dsts' should be freed in case of any error.

  Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x502378 in realloc (/tests/ovstest+0x502378)
    #1 0x622826 in xrealloc /lib/util.c:149:9
    #2 0x8194f4 in parse_select_action /lib/actions.c:1185:20
    #3 0x814f49 in parse_set_action /lib/actions.c:3499:13
    #4 0x814341 in parse_action /lib/actions.c:3554:9
    #5 0x8139ef in parse_actions /lib/actions.c:3643:14
    #6 0x8136a3 in ovnacts_parse /lib/actions.c:3678:9
    #7 0x813c80 in ovnacts_parse_string /lib/actions.c:3705:5
    #8 0x53a4e8 in test_parse_actions /tests/test-ovn.c:1321:17
    #9 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #10 0x537c75 in test_ovn_main /tests/test-ovn.c:1630:5
    #11 0x54e7a8 in ovs_cmdl_run_command__ /lib/command-line.c:247:17
    #12 0x537359 in main /tests/ovstest.c:133:9
    #13 0x7f9ce05ba1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Han Zhou <hzhou@ovn.org>
Fixes: 85b3544 ("ovn-controller: A new action "select".")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'parse_ofp_meter_mod_str' allocates space for meter.bands that
should be freed.

  Direct leak of 448 byte(s) in 7 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    #1 0x7523a6 in xmalloc /lib/util.c:138:15
    #2 0x6fd079 in ofpbuf_init /lib/ofpbuf.c:123:26
    #3 0x6cba27 in parse_ofp_meter_mod_str /lib/ofp-meter.c:779:5
    #4 0x5705b8 in add_meter_string /controller/ofctrl.c:1674:19
    #5 0x56f736 in ofctrl_put /controller/ofctrl.c:2105:13
    #6 0x59aebb in main /controller/ovn-controller.c:2627:25
    #7 0x7f07873251a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

CC: Guoshuai Li <ligs@dtdream.com>
Fixes: c25094b ("ovn: OVN Support QoS meter")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request Nov 22, 2020
'smap_clear()' doesn't free allocated memory, but 'smap_clone()'
re-initializes hash map clearing internal pointers and leaking
this memory.  'smap_destroy()' should be used instead.

Also, all the records and array of datapaths should be freed on
destruction of a cache entry.

  Direct leak of 16 byte(s) in 2 object(s) allocated from:
    #0 0x5211c7 in calloc (/controller/ovn-controller+0x5211c7)
    #1 0x752364 in xcalloc /lib/util.c:121:31
    #2 0x576e76 in sync_dns_cache /controller/pinctrl.c:2517:25
    #3 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    #4 0x59b06c in main /controller/ovn-controller.c:2642:25
    #5 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

  Indirect leak of 26 byte(s) in 2 object(s) allocated from:
    #0 0x52100f in malloc (/controller/ovn-controller+0x52100f)
    #1 0x7523d6 in xmalloc /lib/util.c:138:15
    #2 0x7524a8 in xmemdup0 /lib/util.c:168:15
    #3 0x73d8fc in smap_clone /lib/smap.c:314:45
    #4 0x576e2f in sync_dns_cache /controller/pinctrl.c:2513:13
    #5 0x5758fb in pinctrl_run /controller/pinctrl.c:3158:5
    #6 0x59b06c in main /controller/ovn-controller.c:2642:25
    #7 0x7fb570fc11a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

Fixes: 6b72068 ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
dceara pushed a commit to dceara/ovn that referenced this pull request Mar 2, 2023
Nothing is being freed wherever we are calling
ctl_fatal which is fine because the program is
about to shutdown anyway however one of the
leaks was caught by address sanitizer.
Fix most of the leaks that are happening before
call to ctl_fatal.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x568d70 in __interceptor_realloc.part.0 asan_malloc_linux.cpp.o
    ovn-org#1 0xa530a5 in xrealloc__ /workspace/ovn/ovs/lib/util.c:147:9
    ovn-org#2 0xa530a5 in xrealloc /workspace/ovn/ovs/lib/util.c:179:12
    ovn-org#3 0xa530a5 in x2nrealloc /workspace/ovn/ovs/lib/util.c:239:12
    ovn-org#4 0xa2ee57 in svec_expand /workspace/ovn/ovs/lib/svec.c:92:23
    ovn-org#5 0xa2ef6e in svec_terminate /workspace/ovn/ovs/lib/svec.c:116:5
    ovn-org#6 0x82c117 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:98:5
    ovn-org#7 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#8 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07) (BuildId: 3a287416f70de43f52382f0336980c876f655f90)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c041 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:91:5
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c0b6 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:96:9
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 577a797)
dceara pushed a commit to dceara/ovn that referenced this pull request Mar 2, 2023
Nothing is being freed wherever we are calling
ctl_fatal which is fine because the program is
about to shutdown anyway however one of the
leaks was caught by address sanitizer.
Fix most of the leaks that are happening before
call to ctl_fatal.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x568d70 in __interceptor_realloc.part.0 asan_malloc_linux.cpp.o
    ovn-org#1 0xa530a5 in xrealloc__ /workspace/ovn/ovs/lib/util.c:147:9
    ovn-org#2 0xa530a5 in xrealloc /workspace/ovn/ovs/lib/util.c:179:12
    ovn-org#3 0xa530a5 in x2nrealloc /workspace/ovn/ovs/lib/util.c:239:12
    ovn-org#4 0xa2ee57 in svec_expand /workspace/ovn/ovs/lib/svec.c:92:23
    ovn-org#5 0xa2ef6e in svec_terminate /workspace/ovn/ovs/lib/svec.c:116:5
    ovn-org#6 0x82c117 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:98:5
    ovn-org#7 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#8 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07) (BuildId: 3a287416f70de43f52382f0336980c876f655f90)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c041 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:91:5
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c0b6 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:96:9
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 577a797)
dceara pushed a commit to dceara/ovn that referenced this pull request Mar 2, 2023
Nothing is being freed wherever we are calling
ctl_fatal which is fine because the program is
about to shutdown anyway however one of the
leaks was caught by address sanitizer.
Fix most of the leaks that are happening before
call to ctl_fatal.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x568d70 in __interceptor_realloc.part.0 asan_malloc_linux.cpp.o
    ovn-org#1 0xa530a5 in xrealloc__ /workspace/ovn/ovs/lib/util.c:147:9
    ovn-org#2 0xa530a5 in xrealloc /workspace/ovn/ovs/lib/util.c:179:12
    ovn-org#3 0xa530a5 in x2nrealloc /workspace/ovn/ovs/lib/util.c:239:12
    ovn-org#4 0xa2ee57 in svec_expand /workspace/ovn/ovs/lib/svec.c:92:23
    ovn-org#5 0xa2ef6e in svec_terminate /workspace/ovn/ovs/lib/svec.c:116:5
    ovn-org#6 0x82c117 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:98:5
    ovn-org#7 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#8 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07) (BuildId: 3a287416f70de43f52382f0336980c876f655f90)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c041 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:91:5
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c0b6 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:96:9
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 577a797)
dceara pushed a commit to dceara/ovn that referenced this pull request Mar 2, 2023
Nothing is being freed wherever we are calling
ctl_fatal which is fine because the program is
about to shutdown anyway however one of the
leaks was caught by address sanitizer.
Fix most of the leaks that are happening before
call to ctl_fatal.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x568d70 in __interceptor_realloc.part.0 asan_malloc_linux.cpp.o
    ovn-org#1 0xa530a5 in xrealloc__ /workspace/ovn/ovs/lib/util.c:147:9
    ovn-org#2 0xa530a5 in xrealloc /workspace/ovn/ovs/lib/util.c:179:12
    ovn-org#3 0xa530a5 in x2nrealloc /workspace/ovn/ovs/lib/util.c:239:12
    ovn-org#4 0xa2ee57 in svec_expand /workspace/ovn/ovs/lib/svec.c:92:23
    ovn-org#5 0xa2ef6e in svec_terminate /workspace/ovn/ovs/lib/svec.c:116:5
    ovn-org#6 0x82c117 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:98:5
    ovn-org#7 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#8 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07) (BuildId: 3a287416f70de43f52382f0336980c876f655f90)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c041 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:91:5
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07)
    ovn-org#1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    ovn-org#2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    ovn-org#3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    ovn-org#4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    ovn-org#5 0x82c0b6 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:96:9
    ovn-org#6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    ovn-org#7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 577a797)
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Jun 29, 2023
…eletion.

When multiple LSP deletions are handled in incremental processing, if it
hits a LSP that can't be incrementally processed after incrementally
processing some LSP deletions, it falls back to recompute without
destroying the ovn_port objects that are already handled in the handler,
resulting in memory leaks. See example below, which is detected by the
new test case added by this patch when running with address sanitizer.

=======================

Indirect leak of 936 byte(s) in 3 object(s) allocated from:
    #0 0x55bce7 in calloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bce7)
    ovn-org#1 0x773f4e in xcalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:121:31
    ovn-org#2 0x773f4e in xzalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:131:12
    ovn-org#3 0x773f4e in xzalloc /home/hanzhou/src/ovs/_build/../lib/util.c:165:12
    ovn-org#4 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5
    ovn-org#5 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5
    ovn-org#6 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17
    ovn-org#7 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14
    ovn-org#8 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9
    ovn-org#9 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9
    ovn-org#10 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33
    ovn-org#11 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1)

Indirect leak of 204 byte(s) in 3 object(s) allocated from:
    #0 0x55bea8 in realloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bea8)
    ovn-org#1 0x773c7d in xrealloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:147:9
    ovn-org#2 0x773c7d in xrealloc /home/hanzhou/src/ovs/_build/../lib/util.c:179:12
    ovn-org#3 0x614bd4 in extract_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:228:12
    ovn-org#4 0x614bd4 in extract_lsp_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:243:20
    ovn-org#5 0x5c8d90 in parse_lsp_addrs /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2468:21
    ovn-org#6 0x5b2ebf in join_logical_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2594:13
    ovn-org#7 0x5b2ebf in build_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:4711:5
    ovn-org#8 0x5b2ebf in ovnnb_db_run /home/hanzhou/src/ovn/_build_as/../northd/northd.c:17376:5
    ovn-org#9 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5
    ovn-org#10 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5
    ovn-org#11 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17
    ovn-org#12 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14
    ovn-org#13 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9
    ovn-org#14 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9
    ovn-org#15 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33
    ovn-org#16 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1)
...

Fixes: b337750 ("northd: Incremental processing of VIF changes in 'northd' node.")
Reported-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
hzhou8 added a commit that referenced this pull request Jun 30, 2023
…eletion.

When multiple LSP deletions are handled in incremental processing, if it
hits a LSP that can't be incrementally processed after incrementally
processing some LSP deletions, it falls back to recompute without
destroying the ovn_port objects that are already handled in the handler,
resulting in memory leaks. See example below, which is detected by the
new test case added by this patch when running with address sanitizer.

=======================

Indirect leak of 936 byte(s) in 3 object(s) allocated from:
    #0 0x55bce7 in calloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bce7)
    #1 0x773f4e in xcalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:121:31
    #2 0x773f4e in xzalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:131:12
    #3 0x773f4e in xzalloc /home/hanzhou/src/ovs/_build/../lib/util.c:165:12
    #4 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5
    #5 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5
    #6 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17
    #7 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14
    #8 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9
    #9 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9
    #10 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33
    #11 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1)

Indirect leak of 204 byte(s) in 3 object(s) allocated from:
    #0 0x55bea8 in realloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bea8)
    #1 0x773c7d in xrealloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:147:9
    #2 0x773c7d in xrealloc /home/hanzhou/src/ovs/_build/../lib/util.c:179:12
    #3 0x614bd4 in extract_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:228:12
    #4 0x614bd4 in extract_lsp_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:243:20
    #5 0x5c8d90 in parse_lsp_addrs /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2468:21
    #6 0x5b2ebf in join_logical_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2594:13
    #7 0x5b2ebf in build_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:4711:5
    #8 0x5b2ebf in ovnnb_db_run /home/hanzhou/src/ovn/_build_as/../northd/northd.c:17376:5
    #9 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5
    #10 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5
    #11 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17
    #12 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14
    #13 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9
    #14 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9
    #15 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33
    #16 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1)
...

Fixes: b337750 ("northd: Incremental processing of VIF changes in 'northd' node.")
Reported-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Han Zhou <hzhou@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ales Musil <amusil@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit to dceara/ovn that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      ovn-org#1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      ovn-org#2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      ovn-org#3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      ovn-org#4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      ovn-org#5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      ovn-org#6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      ovn-org#7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      ovn-org#8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      ovn-org#9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
dceara added a commit that referenced this pull request Jul 14, 2023
It's specified in RFC 8415.  This also avoids having to free/realloc the
pfd->uuid.data memory.  That part was not correct anyway and was flagged
by ASAN as a memleak:

  Direct leak of 42 byte(s) in 3 object(s) allocated from:
      #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162)
      #1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15
      #2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12
      #3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20
      #4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9
      #5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9
      #6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9
      #7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17
      #8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12
      #9 0x7faa30194b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

Fixes: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Mar 28, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in ovn-org#2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of ovn-org#1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (ovn-org#2.1/ovn-org#2.2)
Otherwise, during the cleanup in ovn-org#3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Apr 12, 2024
In some scenarios, op->list is not attached anywhere, which makes
attempts to detach it trigger ubsan failure.

ovn/ovs/include/openvswitch/list.h:252:17: runtime error: member access
within null pointer of type 'struct ovs_list'

 #0 0x?? in ovs_list_remove ovn/ovs/include/openvswitch/list.h:252:17
 ovn-org#1 0x?? in ovn_port_allocate_key ovn/northd/northd.c:4021:13
 ovn-org#2 0x?? in ls_port_init ovn/northd/northd.c:4321:10
 ovn-org#3 0x?? in ls_port_create ovn/northd/northd.c:4342:10
 ovn-org#4 0x?? in ls_handle_lsp_changes ovn/northd/northd.c:4511:18
 ovn-org#5 0x?? in northd_handle_ls_changes ovn/northd/northd.c:4655:14
 ovn-org#6 0x?? in northd_nb_logical_switch_handler ovn/northd/en-northd.c:150:

This patch makes northd use op->list only as a temporary means for
build_ports logic to track ports that are persisted in both, nb, or sb
only. Now build_ports will always detach ops once done.

Now that op->list is never left attached to a list, we can remove
ovs_list_remove calls for it elsewhere, including where op was never
attached in the first place.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
dceara pushed a commit to dceara/ovn that referenced this pull request Apr 19, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in ovn-org#2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of ovn-org#1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (ovn-org#2.1/ovn-org#2.2)
Otherwise, during the cleanup in ovn-org#3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Apr 22, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in ovn-org#2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of ovn-org#1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (ovn-org#2.1/ovn-org#2.2)
Otherwise, during the cleanup in ovn-org#3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Apr 26, 2024
In some scenarios, op->list is not attached anywhere, which makes
attempts to detach it trigger ubsan failure.

ovn/ovs/include/openvswitch/list.h:252:17: runtime error: member access
within null pointer of type 'struct ovs_list'

 #0 0x?? in ovs_list_remove ovn/ovs/include/openvswitch/list.h:252:17
 ovn-org#1 0x?? in ovn_port_allocate_key ovn/northd/northd.c:4021:13
 ovn-org#2 0x?? in ls_port_init ovn/northd/northd.c:4321:10
 ovn-org#3 0x?? in ls_port_create ovn/northd/northd.c:4342:10
 ovn-org#4 0x?? in ls_handle_lsp_changes ovn/northd/northd.c:4511:18
 ovn-org#5 0x?? in northd_handle_ls_changes ovn/northd/northd.c:4655:14
 ovn-org#6 0x?? in northd_nb_logical_switch_handler ovn/northd/en-northd.c:150:

This patch makes northd use op->list only as a temporary means for
build_ports logic to track ports that are persisted in both, nb, or sb
only. Now build_ports will always detach ops once done.

Now that op->list is never left attached to a list, we can remove
ovs_list_remove calls for it elsewhere, including where op was never
attached in the first place.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
numansiddique pushed a commit that referenced this pull request Apr 30, 2024
In some scenarios, op->list is not attached anywhere, which makes
attempts to detach it trigger ubsan failure.

ovn/ovs/include/openvswitch/list.h:252:17: runtime error: member access
within null pointer of type 'struct ovs_list'

 #0 0x?? in ovs_list_remove ovn/ovs/include/openvswitch/list.h:252:17
 #1 0x?? in ovn_port_allocate_key ovn/northd/northd.c:4021:13
 #2 0x?? in ls_port_init ovn/northd/northd.c:4321:10
 #3 0x?? in ls_port_create ovn/northd/northd.c:4342:10
 #4 0x?? in ls_handle_lsp_changes ovn/northd/northd.c:4511:18
 #5 0x?? in northd_handle_ls_changes ovn/northd/northd.c:4655:14
 #6 0x?? in northd_nb_logical_switch_handler ovn/northd/en-northd.c:150:

This patch makes northd use op->list only as a temporary means for
build_ports logic to track ports that are persisted in both, nb, or sb
only. Now build_ports will always detach ops once done.

Now that op->list is never left attached to a list, we can remove
ovs_list_remove calls for it elsewhere, including where op was never
attached in the first place.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
Acked-by: Mark Michelson <mmichels@redhat.com>
numansiddique pushed a commit that referenced this pull request May 9, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in #3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique pushed a commit that referenced this pull request May 10, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in #3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 2609cd9)
numansiddique pushed a commit that referenced this pull request May 10, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in #3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 2609cd9)
numansiddique pushed a commit that referenced this pull request May 10, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in #3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 2609cd9)
numansiddique pushed a commit that referenced this pull request May 10, 2024
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in #3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 2609cd9)
ovsrobot pushed a commit to ovsrobot/ovn that referenced this pull request Aug 19, 2024
Commit 8d13579 creates a chassisredirect port for a logical switch
port (of type 'router') in certain scenarios and 'op->nbsp' can be NULL.
The following crash is reported by Sanitizer.

==84927==ERROR: AddressSanitizer: SEGV on unknown address
==84927==The signal is caused by a READ memory access.
==84927==Hint: address points to the zero page.
    #0 0x57ab3854f04a in hmap_first_with_hash ovn/ovs/./include/openvswitch/hmap.h:360:38
    ovn-org#1 0x57ab3854fedf in smap_find__ ovn/ovs/lib/smap.c:421:5
    ovn-org#2 0x57ab3854f7b8 in smap_get_node ovn/ovs/lib/smap.c:217:12
    ovn-org#3 0x57ab3854f74f in smap_get_def ovn/ovs/lib/smap.c:208:30
    ovn-org#4 0x57ab3854f726 in smap_get ovn/ovs/lib/smap.c:200:12
    ovn-org#5 0x57ab3854f862 in smap_get_int ovn/ovs/lib/smap.c:240:25
    ovn-org#6 0x57ab383222eb in ovn_port_assign_requested_tnl_id ovn/northd/northd.c:4372:27
    ovn-org#7 0x57ab383072fa in build_ports ovn/northd/northd.c:4454:9
    ovn-org#8 0x57ab38301457 in ovnnb_db_run ovn/northd/northd.c:18023:5
    ovn-org#9 0x57ab3841d99e in en_northd_run ovn/northd/en-northd.c:137:5
    ovn-org#10 0x57ab384599b2 in engine_recompute ovn/lib/inc-proc-eng.c:411:5
    ovn-org#11 0x57ab38459d6e in engine_run_node ovn/lib/inc-proc-eng.c:473:9
    ovn-org#12 0x57ab38459ec3 in engine_run ovn/lib/inc-proc-eng.c:524:9
    ovn-org#13 0x57ab38430c5d in inc_proc_northd_run ovn/northd/inc-proc-northd.c:420:5
    ovn-org#14 0x57ab3841bb2f in main ovn/northd/ovn-northd.c:970:32

This patch fixes this issue.  It also corrects some typo introduced by
the commit - changes the reference from "chassisresident" to
"chassisredirect".

Fixes: 8d13579 ("Add support for centralize routing for distributed gw ports.")
Reported-by: Felix Huettner <felix.huettner@mail.schwarz>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-August/416264.html
CC: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
numansiddique added a commit that referenced this pull request Aug 20, 2024
Commit 8d13579 creates a chassisredirect port for a logical switch
port (of type 'router') in certain scenarios and 'op->nbsp' can be NULL.
The following crash is reported by Sanitizer.

==84927==ERROR: AddressSanitizer: SEGV on unknown address
==84927==The signal is caused by a READ memory access.
==84927==Hint: address points to the zero page.
    #0 0x57ab3854f04a in hmap_first_with_hash ovn/ovs/./include/openvswitch/hmap.h:360:38
    #1 0x57ab3854fedf in smap_find__ ovn/ovs/lib/smap.c:421:5
    #2 0x57ab3854f7b8 in smap_get_node ovn/ovs/lib/smap.c:217:12
    #3 0x57ab3854f74f in smap_get_def ovn/ovs/lib/smap.c:208:30
    #4 0x57ab3854f726 in smap_get ovn/ovs/lib/smap.c:200:12
    #5 0x57ab3854f862 in smap_get_int ovn/ovs/lib/smap.c:240:25
    #6 0x57ab383222eb in ovn_port_assign_requested_tnl_id ovn/northd/northd.c:4372:27
    #7 0x57ab383072fa in build_ports ovn/northd/northd.c:4454:9
    #8 0x57ab38301457 in ovnnb_db_run ovn/northd/northd.c:18023:5
    #9 0x57ab3841d99e in en_northd_run ovn/northd/en-northd.c:137:5
    #10 0x57ab384599b2 in engine_recompute ovn/lib/inc-proc-eng.c:411:5
    #11 0x57ab38459d6e in engine_run_node ovn/lib/inc-proc-eng.c:473:9
    #12 0x57ab38459ec3 in engine_run ovn/lib/inc-proc-eng.c:524:9
    #13 0x57ab38430c5d in inc_proc_northd_run ovn/northd/inc-proc-northd.c:420:5
    #14 0x57ab3841bb2f in main ovn/northd/ovn-northd.c:970:32

This patch fixes this issue.  It also corrects some typo introduced by
the commit - changes the reference from "chassisresident" to
"chassisredirect".

Fixes: 8d13579 ("Add support for centralize routing for distributed gw ports.")
Reported-by: Felix Huettner <felix.huettner@mail.schwarz>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-August/416264.html
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
numansiddique added a commit that referenced this pull request Aug 20, 2024
Commit 8d13579 creates a chassisredirect port for a logical switch
port (of type 'router') in certain scenarios and 'op->nbsp' can be NULL.
The following crash is reported by Sanitizer.

==84927==ERROR: AddressSanitizer: SEGV on unknown address
==84927==The signal is caused by a READ memory access.
==84927==Hint: address points to the zero page.
    #0 0x57ab3854f04a in hmap_first_with_hash ovn/ovs/./include/openvswitch/hmap.h:360:38
    #1 0x57ab3854fedf in smap_find__ ovn/ovs/lib/smap.c:421:5
    #2 0x57ab3854f7b8 in smap_get_node ovn/ovs/lib/smap.c:217:12
    #3 0x57ab3854f74f in smap_get_def ovn/ovs/lib/smap.c:208:30
    #4 0x57ab3854f726 in smap_get ovn/ovs/lib/smap.c:200:12
    #5 0x57ab3854f862 in smap_get_int ovn/ovs/lib/smap.c:240:25
    #6 0x57ab383222eb in ovn_port_assign_requested_tnl_id ovn/northd/northd.c:4372:27
    #7 0x57ab383072fa in build_ports ovn/northd/northd.c:4454:9
    #8 0x57ab38301457 in ovnnb_db_run ovn/northd/northd.c:18023:5
    #9 0x57ab3841d99e in en_northd_run ovn/northd/en-northd.c:137:5
    #10 0x57ab384599b2 in engine_recompute ovn/lib/inc-proc-eng.c:411:5
    #11 0x57ab38459d6e in engine_run_node ovn/lib/inc-proc-eng.c:473:9
    #12 0x57ab38459ec3 in engine_run ovn/lib/inc-proc-eng.c:524:9
    #13 0x57ab38430c5d in inc_proc_northd_run ovn/northd/inc-proc-northd.c:420:5
    #14 0x57ab3841bb2f in main ovn/northd/ovn-northd.c:970:32

This patch fixes this issue.  It also corrects some typo introduced by
the commit - changes the reference from "chassisresident" to
"chassisredirect".

Fixes: 8d13579 ("Add support for centralize routing for distributed gw ports.")
Reported-by: Felix Huettner <felix.huettner@mail.schwarz>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-August/416264.html
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 9fbda4a)
LorenzoBianconi pushed a commit to LorenzoBianconi/ovn that referenced this pull request Aug 28, 2024
Commit 8d13579 creates a chassisredirect port for a logical switch
port (of type 'router') in certain scenarios and 'op->nbsp' can be NULL.
The following crash is reported by Sanitizer.

==84927==ERROR: AddressSanitizer: SEGV on unknown address
==84927==The signal is caused by a READ memory access.
==84927==Hint: address points to the zero page.
    #0 0x57ab3854f04a in hmap_first_with_hash ovn/ovs/./include/openvswitch/hmap.h:360:38
    ovn-org#1 0x57ab3854fedf in smap_find__ ovn/ovs/lib/smap.c:421:5
    ovn-org#2 0x57ab3854f7b8 in smap_get_node ovn/ovs/lib/smap.c:217:12
    ovn-org#3 0x57ab3854f74f in smap_get_def ovn/ovs/lib/smap.c:208:30
    ovn-org#4 0x57ab3854f726 in smap_get ovn/ovs/lib/smap.c:200:12
    ovn-org#5 0x57ab3854f862 in smap_get_int ovn/ovs/lib/smap.c:240:25
    ovn-org#6 0x57ab383222eb in ovn_port_assign_requested_tnl_id ovn/northd/northd.c:4372:27
    ovn-org#7 0x57ab383072fa in build_ports ovn/northd/northd.c:4454:9
    ovn-org#8 0x57ab38301457 in ovnnb_db_run ovn/northd/northd.c:18023:5
    ovn-org#9 0x57ab3841d99e in en_northd_run ovn/northd/en-northd.c:137:5
    ovn-org#10 0x57ab384599b2 in engine_recompute ovn/lib/inc-proc-eng.c:411:5
    ovn-org#11 0x57ab38459d6e in engine_run_node ovn/lib/inc-proc-eng.c:473:9
    ovn-org#12 0x57ab38459ec3 in engine_run ovn/lib/inc-proc-eng.c:524:9
    ovn-org#13 0x57ab38430c5d in inc_proc_northd_run ovn/northd/inc-proc-northd.c:420:5
    ovn-org#14 0x57ab3841bb2f in main ovn/northd/ovn-northd.c:970:32

This patch fixes this issue.  It also corrects some typo introduced by
the commit - changes the reference from "chassisresident" to
"chassisredirect".

Fixes: 8d13579 ("Add support for centralize routing for distributed gw ports.")
Reported-by: Felix Huettner <felix.huettner@mail.schwarz>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-August/416264.html
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants