summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Set release date for 2.5.9.v2.5.9Justin Pettit2019-09-062-2/+3
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* datapath: Clear the L4 portion of the key for "later" fragmentsJustin Pettit2019-09-041-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | Upstream commit: commit 0754b4e8cdf3eec6e4122e79af26ed9bab20f8f8 Author: Justin Pettit <jpettit@ovn.org> Date: Tue Aug 27 07:58:10 2019 -0700 openvswitch: Clear the L4 portion of the key for "later" fragments. Only the first fragment in a datagram contains the L4 headers. When the Open vSwitch module parses a packet, it always sets the IP protocol field in the key, but can only set the L4 fields on the first fragment. The original behavior would not clear the L4 portion of the key, so garbage values would be sent in the key for "later" fragments. This patch clears the L4 fields in that circumstance to prevent sending those garbage values as part of the upcall. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* datapath: Properly set L4 keys on "later" IP fragmentsGreg Rose2019-09-043-52/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Upstream commit: commit ad06a566e118e57b852cab5933dbbbaebb141de3 Author: Greg Rose <gvrose8192@gmail.com> Date: Tue Aug 27 07:58:09 2019 -0700 openvswitch: Properly set L4 keys on "later" IP fragments When IP fragments are reassembled before being sent to conntrack, the key from the last fragment is used. Unless there are reordering issues, the last fragment received will not contain the L4 ports, so the key for the reassembled datagram won't contain them. This patch updates the key once we have a reassembled datagram. The handle_fragments() function works on L3 headers so we pull the L3/L4 flow key update code from key_extract into a new function 'key_extract_l3l4'. Then we add a another new function ovs_flow_key_update_l3l4() and export it so that it is accessible by handle_fragments() for conntrack packet reassembly. Co-authored-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* ofproto-dpif: Fix using uninitialised memory in user_action_cookie.Ilya Maximets2019-08-242-16/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Designated initializers are not suitable for initializing non-packed structures and unions which are subjects for comparison by memcmp(). Whole memory for 'struct user_action_cookie' must be explicitly cleared before using because it will be copied with memcpy and later compared by memcmp in ofpbuf_equal(). Few issues found be valgrind: Thread 13 revalidator11: Conditional jump or move depends on uninitialised value(s) at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so) by 0x9D4404: ofpbuf_equal (ofpbuf.h:273) by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219) by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2286) by 0x9D62AC: revalidate (ofproto-dpif-upcall.c:2685) by 0x9D62AC: udpif_revalidator (ofproto-dpif-upcall.c:942) by 0xA9C732: ovsthread_wrapper (ovs-thread.c:383) by 0x5FF86DA: start_thread (pthread_create.c:463) by 0x6AF488E: clone (clone.S:95) Uninitialised value was created by a stack allocation at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062) Thread 11 revalidator16: Conditional jump or move depends on uninitialised value(s) at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so) by 0x9D4404: ofpbuf_equal (ofpbuf.h:273) by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2220) by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2287) by 0x9D62BC: revalidate (ofproto-dpif-upcall.c:2686) by 0x9D62BC: udpif_revalidator (ofproto-dpif-upcall.c:942) by 0xA9C6D2: ovsthread_wrapper (ovs-thread.c:383) by 0x5FF86DA: start_thread (pthread_create.c:463) by 0x6AF488E: clone (clone.S:95) Uninitialised value was created by a stack allocation at 0x9DC4E0: compose_sflow_action (ofproto-dpif-xlate.c:3211) The struct was never marked as 'packed', however it was manually adjusted to be so in practice. Old IPFIX related commit first made the structure non-contiguous. Commit 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.") added uninitialized parts of the additional union space and the next one introduced new holes between structure fields for all cases. CC: Justin Pettit <jpettit@ovn.org> Fixes: 8b7ea2d48033 ("Extend OVS IPFIX exporter to export tunnel headers") Fixes: 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.") Fixes: fcb9579be3c7 ("ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org>
* stream-ssl: Fix crash on NULL private key and valid certificate.Ilya Maximets2019-06-281-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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) #3 0x7ff759473c5a in BIO_ctrl (/lib64/libcrypto.so.1.1+0xadc5a) #4 0x7ff7598decc1 in SSL_CTX_use_certificate_file (/lib64/libssl.so.1.1+0x40cc1) #5 0x4dbaa7 in stream_ssl_set_certificate_file__ lib/stream-ssl.c:1170 #6 0x4dca2e in stream_ssl_set_key_and_cert lib/stream-ssl.c:1216 #7 0x4146b2 in reconfigure_ssl ovsdb/ovsdb-server.c:1254 #8 0x409c83 in main ovsdb/ovsdb-server.c:368 #9 0x7ff758233812 in __libc_start_main #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: 6f1e91b1d7c0 ("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>
* datapath: fix flow actions reallocationAndrea Righi2019-04-151-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Upstream commit: commit f28cd2af22a0c134e4aa1c64a70f70d815d473fb Author: Andrea Righi <andrea.righi@canonical.com> Date: Thu Mar 28 07:36:00 2019 +0100 openvswitch: fix flow actions reallocation The flow action buffer can be resized if it's not big enough to contain all the requested flow actions. However, this resize doesn't take into account the new requested size, the buffer is only increased by a factor of 2x. This might be not enough to contain the new data, causing a buffer overflow, for example: [ 42.044472] ============================================================================= [ 42.045608] BUG kmalloc-96 (Not tainted): Redzone overwritten [ 42.046415] ----------------------------------------------------------------------------- [ 42.047715] Disabling lock debugging due to kernel taint [ 42.047716] INFO: 0x8bf2c4a5-0x720c0928. First byte 0x0 instead of 0xcc [ 42.048677] INFO: Slab 0xbc6d2040 objects=29 used=18 fp=0xdc07dec4 flags=0x2808101 [ 42.049743] INFO: Object 0xd53a3464 @offset=2528 fp=0xccdcdebb [ 42.050747] Redzone 76f1b237: cc cc cc cc cc cc cc cc ........ [ 42.051839] Object d53a3464: 6b 6b 6b 6b 6b 6b 6b 6b 0c 00 00 00 6c 00 00 00 kkkkkkkk....l... [ 42.053015] Object f49a30cc: 6c 00 0c 00 00 00 00 00 00 00 00 03 78 a3 15 f6 l...........x... [ 42.054203] Object acfe4220: 20 00 02 00 ff ff ff ff 00 00 00 00 00 00 00 00 ............... [ 42.055370] Object 21024e91: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ [ 42.056541] Object 070e04c3: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ [ 42.057797] Object 948a777a: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ [ 42.059061] Redzone 8bf2c4a5: 00 00 00 00 .... [ 42.060189] Padding a681b46e: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ Fix by making sure the new buffer is properly resized to contain all the requested data. BugLink: https://bugs.launchpad.net/bugs/1813244 Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Cc: Andrea Righi <andrea.righi@canonical.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* Prepare for 2.5.9.Justin Pettit2019-04-103-1/+10
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* Set release date for 2.5.8.v2.5.8Justin Pettit2019-04-102-2/+3
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ifupdown.sh: Add missing "--may-exist" optionGeorge Diamantopoulos2019-03-221-1/+1
| | | | | | | | | | | | | | | | | | | | | The ifupdown.sh script passes the --may-exist option to ovs-vsctl invocations in order for it to exit without failing if the device to be added already exists. This holds true for all cases of adding objects to ovs-vswitchd except for when configuring a bond interface. This patch adds the --may-exist option to the missing statement, which suppresses the logging of such errors in syslog. Additionally, running the unpatched version of this script when the bond interface already exists appears to break networking with some versions of ifupdown found in debian testing (0.8.35), where the service won't start up properly because of the aforementioned errors. Signed-off-by: George Diamantopoulos <georgediam@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* travis: Automatically recheck failed tests.Ben Pfaff2019-03-201-1/+1
| | | | | | | | | | This should make the automatic testsuite more reliable on Travis. It's better to fix tests to be more reliable, of course, but in practie it's difficult to make all of them 100% reliable. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Simon Horman <simon.horman@netronome.com>
* tests: Add ability to automatically rerun failed tests.Ben Pfaff2019-03-207-12/+13
| | | | | | | | | A lot of packaging was doing this already, so this simplifies their implementation. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Simon Horman <simon.horman@netronome.com>
* travis: Remove 'sudo' configuration.Ilya Maximets2019-03-061-2/+0
| | | | | | | | | | | | | | | | Since TravisCI migrated jobs from containers to VMs, 'sudo' is always available. Setting 'sudo: false' is misleading because it makes no effect. https://docs.travis-ci.com/user/reference/trusty/#container-based-infrastructure "Container-based infrastructure is currently being deprecated. Please remove any sudo: false keys in your .travis.yml file to use the default fully-virtualized Linux infrastructure instead." Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* rconn: Avoid occasional immediate connection failures.Ben Pfaff2019-03-011-2/+2
| | | | | | | | | | | The rconn connection timer measures time on the granularity of seconds, which means that sometimes the actual timeout can be just a millsecond or so, which led to occasional immediate connection failures from ovs-ofctl. VMware-BZ: #2295760 Fixes: 476d2551abd2 ("rconn: Introduce new invariant to fix assertion failure in corner case.") Reported-by: Ken Ajiro <ken-ajiro@xr.jp.nec.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* Prepare for 2.5.8.Justin Pettit2019-02-213-1/+10
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org>
* Set release date for 2.5.7.v2.5.7Justin Pettit2019-02-212-2/+3
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org>
* odp-util: Stop parse odp actions if nlattr is overflowYifeng Sun2019-02-041-0/+4
| | | | | | | | | `encap = nl_msg_start_nested(key, OVS_KEY_ATTR_ENCAP)` ensures that key->size >= (encap + NLA_HDRLEN), so the `if` statement is safe. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11306 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* stt: Fix return code during xmit.Aaron Conole2019-01-241-1/+1
| | | | | | | | | | | | | | | | | | In the case of an error, return the error code as opposed to NETDEV_TX_OK. Caught by compiler warning: /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c: In function =E2=80= =98ovs_stt_xmit=E2=80=99: /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c:1005:6: warning: var= iable =E2=80=98err=E2=80=99 set but not used [-Wunused-but-set-variable] int err; ^ Signed-off-by: Aaron Conole <aconole@redhat.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* netdev-linux: Fix function argument order in sfq_tc_load().Ben Pfaff2019-01-171-1/+1
| | | | | | | | sfq_install__() takes quantum before perturb. Acked-by: Justin Pettit <jpettti@ovn.org> Reported-by: shaoke xi <xishaoke.xsk@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* python: Escape backslashes while formatting logs.Ilya Maximets2019-01-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since python version 3.7 (and some 3.6+ versions) regexp engine changed to treat the wrong escape sequences as errors. Previously, if the replace string had something like '\u0000', '\u' was qualified as a bad escape sequence and treated just as a sequence of characters '\' and 'u'. But know this triggers an error: Traceback (most recent call last): File "/usr/lib/python3.7/sre_parse.py", line 1021, in parse_template this = chr(ESCAPES[this][1]) KeyError: '\\u' From the documentation [1]: Unknown escapes consisting of '\' and an ASCII letter in replacement templates for re.sub() were deprecated in Python 3.5, and will now cause an error. [1] https://docs.python.org/3/whatsnew/3.7.html#api-and-feature-removals We need to escape the backslash by another one to keep regexp engine from errors. In case of '\\u000', '\\' is a valid escape sequence and the 'u' is a simple character. To be 100% safe we need to use 're.escape(replace)', but it escapes too many characters making the logs hard to read. This change fixes Python 3 tests on systems with python 3.7. Should be backward compatible. Reported-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* cmap: Fix hashing in cmap_find_protected().Zang MingJie2018-12-241-1/+1
| | | | | | | | | cmap_find_protected calculated wrong h2 hash which causing entries with duplicated id inserted into the cmap. Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-December/047945.html Signed-off-by: Zang MingJie <zealot0630@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* python: Catch setsockopt exceptions for TCP stream.Ilya Maximets2018-12-201-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | 'sock.setsockopt' could throw exceptions. For example, if non-blocking connection failed before the call: Traceback (most recent call last): File "../.././test-ovsdb.py", line 896, in <module> main(sys.argv) File "../.././test-ovsdb.py", line 891, in main func(*args) File "../.././test-ovsdb.py", line 604, in do_idl ovs.stream.Stream.open(r)) File "/root/git_/ovs/python/ovs/stream.py", line 190, in open error, sock = cls._open(suffix, dscp) File "/root/git_/ovs/python/ovs/stream.py", line 744, in _open sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) File "/usr/local/lib/python2.7/socket.py", line 228, in meth return getattr(self._sock,name)(*args) socket.error: [Errno 54] Connection reset by peer This fixes tests on FreeBSD. Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* debian: Install correct vtep-ctl.Ben Pfaff2018-12-031-1/+1
| | | | | | | | | | | | The previous syntax installed the libtool wrapper script instead of the actual binary. This fixes the problem. CC: James Page <james.page@ubuntu.com> Fixes: 3d8dededeaf8 ("debian: Rationalize packaging using new debhelper.") Reported-by: hubo <hubo@jiedaibao.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047625.html Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
* datapath-windows: Fix invalid reference in Buffermgmt.cSairam Venugopal2018-11-151-2/+4
| | | | | | | | | | | | OVS_BUFFER_CONTEXT gets cleared as part of NdisFreeNetBufferListContext function call. This causes an invalid reference error. Found while testing with driver verifier enabled. Signed-off-by: Sairam Venugopal <vsairam@vmware.com> Acked-by: Anand Kumar <kumaranand@vmware.com> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
* bond: Fix LACP fallback to active-backup when recirc is enabled.Ben Pfaff2018-11-051-3/+22
| | | | | | Reported-by: Arun Navasivasakthivelsamy <arunkum.navasiv@nutanix.com> Tested-by: Arun Navasivasakthivelsamy <arunkum.navasiv@nutanix.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* bridge.c: prevent controller connects while flow-restore-waitZak Whittington2018-10-251-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When force-reload-kmod is used, it shows an error when reinstalling tlvs during "Restoring saved flows" step: OFPT_ERROR (xid=0x4): NXTTMFC_ALREADY_MAPPED This is caused by a race condition between the restore script, which calls ofctl, and the connected controllers both adding back the same TLVs. The restore script already sets flow-restore-wait to true while doing flow restoration, and sets it back to false after it is done, and this patch utilizes that fact to prevent the TLV race. It does this by preventing vswitchd from connecting to controllers in the controller table while it is in a flow-restore-wait state. With this patch, when bridge_configure_remotes() calls bridge_get_controllers(), it first checks if flow-restore-wait has been set, and if so, it ignores any controllers in the controller database and sets n_controllers to 0. This solution does preserve the management service controller which is added via bridge_ofproto_controller_for_mgmt() after checking whether we should call bridge_get_controllers() (and thus n_controllers is properly set to 1, etc) VMware-BZ: 2195377 Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* Prepare for 2.5.7.Justin Pettit2018-10-194-1/+16
| | | | Signed-off-by: Justin Pettit <jpettit@ovn.org>
* Set release date for 2.5.6.v2.5.6Justin Pettit2018-10-192-3/+4
| | | | Signed-off-by: Justin Pettit <jpettit@ovn.org>
* test-hash: Fix unaligned pointer value error.Joe Stringer2018-10-161-10/+10
| | | | | | | | | | | | | | | | | | | | Clang 4.0 complains: ../tests/test-hash.c:160:16: error: taking address of packed member 'b' of class or structure 'offset_ovs_u128' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member] in0 = &in0_data.b; Set the bit in the aligned u128 first then copy the contents into the offset u128 so that we don't have to take the address of the non-aligned u128 and pass it to set_bit128. For the 256byte_hash, fix it up so that it's actually testing the 256B hash inside a 32-bit offset u128 as well. Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* odp-execute: Fix broken build with Clang as compiler.Ben Pfaff2018-10-163-5/+5
| | | | | | | | | | | | | | | | | | | | | | | Builds of branch-2.7 have been failing on Travis when Clang is used as compiler due to: ../ofproto/ofproto-dpif.c:2057:46: warning: taking address of packed member 'eth_src' of class or structure 'eth_header' may result in an unaligned pointer value [-Waddress-of-packed-member] netdev_get_etheraddr(ofport->up.netdev, &eth->eth_src); ^~~~~~~~~~~~ ../ofproto/ofproto-dpif.c:2082:50: warning: taking address of packed member 'eth_src' of class or structure 'eth_header' may result in an unaligned pointer value [-Waddress-of-packed-member] netdev_get_etheraddr(ofport->up.netdev, &eth->eth_src); ^~~~~~~~~~~~ On master these don't come up because of commit 1620b7ea68c2 ("packets: Remove unnecessary "packed" annotations."), which removed the packed annotation that causes the warning. This commit applies enough of that commit to make the build pass. Signed-off-by: Ben Pfaff <blp@ovn.org>
* travis: Remove sparse support.Ben Pfaff2018-10-162-10/+2
| | | | | | | "sparse" failed to build with this old branch, see e.g. https://travis-ci.org/openvswitch/ovs/jobs/436851158 Signed-off-by: Ben Pfaff <blp@ovn.org>
* Revert "bridge: Fix ovs-appctl qos/show repeated queue information"Ben Pfaff2018-10-031-1/+0
| | | | | | | | | | | | This reverts commit 6b4d0211e84a ("bridge: Fix ovs-appctl qos/show repeated queue information"), which is no longer necessary now that commit 65f3c34c7417 ("netdev: Properly clear 'details' when iterating in NETDEV_QOS_FOR_EACH.") has been applied. The former commit fixed a symptom of the root cause fixed by the latter. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org>
* netdev: Properly clear 'details' when iterating in NETDEV_QOS_FOR_EACH.Ben Pfaff2018-10-031-1/+2
| | | | | | | | | | | The function comment for netdev_queue_dump_next() said that it cleared its 'detail' argument, but it didn't actually do that, which meant that details could be incorrectly carried along from one queue to the next. Reported-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org>
* bridge: Fix ovs-appctl qos/show repeated queue informationEelco Chaudron2018-10-021-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The patch below would stop qos/show to repeat information from the previous queues. See below an example before and after the fix: Before: $ ovs-appctl qos/show p5p2 QoS: p5p2 linux-htb max-rate: 2428800 Default: burst: 12512 min-rate: 12000 max-rate: 2428800 tx_packets: 0 tx_bytes: 0 tx_errors: 0 Queue 20: burst: 12512 burst: 12512 min-rate: 12000 min-rate: 12000 max-rate: 607200 max-rate: 2428800 tx_packets: 28780 tx_bytes: 43572920 tx_errors: 17611 Queue 10: burst: 12512 burst: 12512 burst: 12512 max-rate: 2428800 max-rate: 607200 max-rate: 2428800 min-rate: 12000 min-rate: 12000 min-rate: 12000 tx_packets: 71751 tx_bytes: 108631014 tx_errors: 18503 After: $ ovs-appctl qos/show p5p2 QoS: p5p2 linux-htb max-rate: 2428800 Default: burst: 12512 min-rate: 12000 max-rate: 2428800 tx_packets: 0 tx_bytes: 0 tx_errors: 0 Queue 20: burst: 12512 min-rate: 12000 max-rate: 607200 tx_packets: 28780 tx_bytes: 43572920 tx_errors: 17611 Queue 10: burst: 12512 min-rate: 12000 max-rate: 2428800 tx_packets: 71751 tx_bytes: 108631014 tx_errors: 18503 Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* lex: Fix buffer overrun parsing overlong hexadecimal constants.Yifeng Sun2018-10-021-6/+8
| | | | | | | | | In previous code, if hexit == 0, then the boundary for 'out' is not checked. This patch fixes it. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ovsdb-client: Fix a bug that uses wrong indexYifeng Sun2018-09-271-2/+2
| | | | | | | This patch fixes the incorrect index to argv. Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* flow: Fix uninitialized flow fields in IPv6 error case.Ben Pfaff2018-09-211-5/+6
| | | | | | | | | | When parse_ipv6_ext_hdrs__() returned false, half a 64-bit word had been pushed into the miniflow and the second half was left uninitialized. This commit fixes the problem. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518 Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* meta-flow: Make "nw_frag" a synonym for "ip_frag".Ben Pfaff2018-09-202-6/+6
| | | | | | | | | | | | Since the time that OVS introduced support for IP fragments, the OVS functions that format flows have used "nw_frag", but the ones that parse flows have expected "ip_frag". Obviously this is a bug and it's a surprise that it's gone so long without anyone reporting the problem. This fixes it and adds a test. Reported-by: Gurucharan Shetty <guru@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Gurucharan Shetty <guru@ovn.org>
* datapath: lisp: Fix uninitialized field in tunnel_cfg.Yunjian Wang2018-09-121-0/+1
| | | | | | | | | The tunnel_cfg had the gro_receive and gro_complete fields uninitialized in function lisp_open(). This caused an uninitialized memory read. Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
* odp-util: Don't attempt to write IPv6 flow label bits that don't exist.Ben Pfaff2018-09-121-0/+1
| | | | | | | | | | | | The ipv6_label field member of struct ovs_key_ipv6 is 32 bits in size, but an IPv6 label is only 20 bits, so the upper 12 bits are not writable and must be 0 in the mask. The code wasn't careful about this so it could try to write them anyway. This commit fixes the problem. Reported-by: nm_r@directbox.com Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047357.html Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* daemon-unix: Use same name for original or restarted children.Ben Pfaff2018-09-061-1/+1
| | | | | | | | | | | | | Linux has an idea of process name that is visible in /proc/$pid/comm. This is "ovs-vswitchd" for a freshly started ovs-vswitchd process. When the monitor code restarted a crash child, it changed it to the empty string. This confused the daemon_is_running check in ovs-lib.in, which checks comm. This commit fixes the problem by setting the program name as comm in newly restarted children. VMware-BZ: #2191724 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Gurucharan Shetty <guru@ovn.org>
* utilities: Drop shebang from bash completion scriptMarkos Chandras2018-08-301-1/+0
| | | | | | | | | | | | | | | | | | This fixes the following warning when building Open vSwitch on the openSUSE Build Service: W: non-executable-script /usr/share/bash-completion/completions/ovs-appctl-bashcomp.bash This text file contains a shebang or is located in a path dedicated for executables, but lacks the executable bits and cannot thus be executed. If the file is meant to be an executable script, add the executable bits, otherwise remove the shebang or move the file elsewhere. The file is meant to be sourced instead of executed, so we can simply drop the shebang. Signed-off-by: Markos Chandras <mchandras@suse.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
* netdev-linux: Avoid division by 0 if kernel reports bad scheduler data.Ben Pfaff2018-08-201-1/+1
| | | | | | | | | | | | | If the kernel reported a value of 0 for the second value in /proc/net/psched, it would cause a division-by-zero fault in read_psched(). I don't know of a kernel that would actually do that, but it's still better to be safe. Found by clang static analyzer. Reported-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* pcap-file: Fix formatting of log message.Ben Pfaff2018-08-031-1/+1
| | | | | Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
* datapath: Don't swap table in nlattr_set() after OVS_ATTR_NESTED is foundStefano Brivio2018-07-301-6/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Upstream commit: commit 72f17baf2352ded6a1d3f4bb2d15da8c678cd2cb Author: Stefano Brivio <sbrivio@redhat.com> Date: Thu May 3 18:13:25 2018 +0200 openvswitch: Don't swap table in nlattr_set() after OVS_ATTR_NESTED is found If an OVS_ATTR_NESTED attribute type is found while walking through netlink attributes, we call nlattr_set() recursively passing the length table for the following nested attributes, if different from the current one. However, once we're done with those sub-nested attributes, we should continue walking through attributes using the current table, instead of using the one related to the sub-nested attributes. For example, given this sequence: 1 OVS_KEY_ATTR_PRIORITY 2 OVS_KEY_ATTR_TUNNEL 3 OVS_TUNNEL_KEY_ATTR_ID 4 OVS_TUNNEL_KEY_ATTR_IPV4_SRC 5 OVS_TUNNEL_KEY_ATTR_IPV4_DST 6 OVS_TUNNEL_KEY_ATTR_TTL 7 OVS_TUNNEL_KEY_ATTR_TP_SRC 8 OVS_TUNNEL_KEY_ATTR_TP_DST 9 OVS_KEY_ATTR_IN_PORT 10 OVS_KEY_ATTR_SKB_MARK 11 OVS_KEY_ATTR_MPLS we switch to the 'ovs_tunnel_key_lens' table on attribute #3, and we don't switch back to 'ovs_key_lens' while setting attributes #9 to #11 in the sequence. As OVS_KEY_ATTR_MPLS evaluates to 21, and the array size of 'ovs_tunnel_key_lens' is 15, we also get this kind of KASan splat while accessing the wrong table: [ 7654.586496] ================================================================== [ 7654.594573] BUG: KASAN: global-out-of-bounds in nlattr_set+0x164/0xde9 [openvswitch] [ 7654.603214] Read of size 4 at addr ffffffffc169ecf0 by task handler29/87430 [ 7654.610983] [ 7654.612644] CPU: 21 PID: 87430 Comm: handler29 Kdump: loaded Not tainted 3.10.0-866.el7.test.x86_64 #1 [ 7654.623030] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016 [ 7654.631379] Call Trace: [ 7654.634108] [<ffffffffb65a7c50>] dump_stack+0x19/0x1b [ 7654.639843] [<ffffffffb53ff373>] print_address_description+0x33/0x290 [ 7654.647129] [<ffffffffc169b37b>] ? nlattr_set+0x164/0xde9 [openvswitch] [ 7654.654607] [<ffffffffb53ff812>] kasan_report.part.3+0x242/0x330 [ 7654.661406] [<ffffffffb53ff9b4>] __asan_report_load4_noabort+0x34/0x40 [ 7654.668789] [<ffffffffc169b37b>] nlattr_set+0x164/0xde9 [openvswitch] [ 7654.676076] [<ffffffffc167ef68>] ovs_nla_get_match+0x10c8/0x1900 [openvswitch] [ 7654.684234] [<ffffffffb61e9cc8>] ? genl_rcv+0x28/0x40 [ 7654.689968] [<ffffffffb61e7733>] ? netlink_unicast+0x3f3/0x590 [ 7654.696574] [<ffffffffc167dea0>] ? ovs_nla_put_tunnel_info+0xb0/0xb0 [openvswitch] [ 7654.705122] [<ffffffffb4f41b50>] ? unwind_get_return_address+0xb0/0xb0 [ 7654.712503] [<ffffffffb65d9355>] ? system_call_fastpath+0x1c/0x21 [ 7654.719401] [<ffffffffb4f41d79>] ? update_stack_state+0x229/0x370 [ 7654.726298] [<ffffffffb4f41d79>] ? update_stack_state+0x229/0x370 [ 7654.733195] [<ffffffffb53fe4b5>] ? kasan_unpoison_shadow+0x35/0x50 [ 7654.740187] [<ffffffffb53fe62a>] ? kasan_kmalloc+0xaa/0xe0 [ 7654.746406] [<ffffffffb53fec32>] ? kasan_slab_alloc+0x12/0x20 [ 7654.752914] [<ffffffffb53fe711>] ? memset+0x31/0x40 [ 7654.758456] [<ffffffffc165bf92>] ovs_flow_cmd_new+0x2b2/0xf00 [openvswitch] [snip] [ 7655.132484] The buggy address belongs to the variable: [ 7655.138226] ovs_tunnel_key_lens+0xf0/0xffffffffffffd400 [openvswitch] [ 7655.145507] [ 7655.147166] Memory state around the buggy address: [ 7655.152514] ffffffffc169eb80: 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa fa [ 7655.160585] ffffffffc169ec00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 7655.168644] >ffffffffc169ec80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa [ 7655.176701] ^ [ 7655.184372] ffffffffc169ed00: fa fa fa fa 00 00 00 00 fa fa fa fa 00 00 00 05 [ 7655.192431] ffffffffc169ed80: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00 [ 7655.200490] ================================================================== Reported-by: Hangbin Liu <liuhangbin@gmail.com> Fixes: 982b52700482 ("openvswitch: Fix mask generation for nested attributes.") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* compat: Initialize IPv4 reassembly secret timerGreg Rose2018-07-272-0/+6
| | | | | | | | | | | | | | | | | | The RHEL 7 kernels expect the secret timer interval to be initialized before calling the inet_frags_init() function. By not initializing it the inet_frags_secret_rebuild() function was running on every tick rather than on the expected interval. This caused occasional panics from page faults when inet_frags_secret_rebuild() would try to rearm a timer from the openvswitch kernel module which had just been removed. Also remove the prior, and now unnecessary, work around. VMware BZ 2094203 Fixes: 595e069a ("compat: Backport IPv4 reassembly.") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
* Revert "flow: Fix buffer overread for crafted IPv6 packets."Justin Pettit2018-07-161-1/+1
| | | | | | | | | | | | | | | | | | This reverts commit 0760bd61a666e9fa866fcb5ed67f48f34895d2f6. This patch was a cherry-pick from a bug fix in the master branch that fixed an overread for IPv6 packets. However, the backport introduced a problem in older branches, since the code path is different. In the master branch, this check is done on the raw packet data, which starts at the beginning of the IPv6 packet. In older branches, this check is done after a call to data_pull(), which subtracts the IPv6 header length from the 'size' variable. This means that valid IPv6 packets aren't being processed since the check thinks they are too long. CC: Ben Pfaff <blp@ovn.org> Fixes: 0760bd61a66 ("flow: Fix buffer overread for crafted IPv6 packets.") Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-By: Lucas Alvares Gomes <lucasagomes@gmail.com>
* flow: Fix buffer overread for crafted IPv6 packets.Ben Pfaff2018-07-091-1/+1
| | | | | | | | | | | | | | | The ipv6_sanity_check() function implemented a check for IPv6 payload length wrong: ip6_plen is the payload length but this function checked whether it was longer than the total length of IPv6 header plus payload. This meant that a packet with a crafted ip6_plen could result in a buffer overread of up to the length of an IPv6 header (40 bytes). The kernel datapath flow extraction code does not obviously have a similar problem. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9287 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Darrell Ball <dlu998@gmail.com>
* ofp-group: Don't assert-fail decoding bad OF1.5 group mod type or command.Ben Pfaff2018-07-061-2/+2
| | | | | | | | | | | | | | When decoding a group mod, the current code validates the group type and command after the whole group mod has been decoded. The OF1.5 decoder, however, tries to use the type and command earlier, when it might still be invalid. This caused an assertion failure (via OVS_NOT_REACHED). This commit fixes the problem. ovs-vswitchd does not enable support for OpenFlow 1.5 by default. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9249 Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofp-actions: Fix buffer overread in decode_LEARN_specs().Ben Pfaff2018-07-051-1/+1
| | | | | | | | The length check was wrong for immediate arguments to "learn" actions. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9047 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* ofp-actions: Avoid buffer overread in BUNDLE action decoding.Ben Pfaff2018-07-051-6/+7
| | | | | | Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9052 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
View Lorry Controller Status