[ovs-git] [openvswitch/ovs] 0c147f: dpif-netlink: Fix using uninitialized info.tc_modi...

Mark Gray noreply at github.com
Tue Apr 20 10:04:52 UTC 2021


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 0c147fb4e57c620205de7a36c614074209248d7b
      https://github.com/openvswitch/ovs/commit/0c147fb4e57c620205de7a36c614074209248d7b
  Author: Yunjian Wang <wangyunjian at huawei.com>
  Date:   2021-04-19 (Mon, 19 Apr 2021)

  Changed paths:
    M lib/dpif-netlink.c

  Log Message:
  -----------
  dpif-netlink: Fix using uninitialized info.tc_modify_flow_deleted in out label.

Before info.tc_modify_flow_deleted is assigned a value, error
processing of other statements goes to the out label. In the
out label, the uninitialized variant is used for condition
determination, which may cause uncertain behavior.

Fixes: 65b84d4a32bd ("dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.")
Signed-off-by: Mengfan Lv <lvmengfan at huawei.com>
Signed-off-by: Yunjian Wang <wangyunjian at huawei.com>
Reviewed-by: Simon Horman <simon.horman at netronome.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: ea71a9d44316199f2b70efe328c649f71223f670
      https://github.com/openvswitch/ovs/commit/ea71a9d44316199f2b70efe328c649f71223f670
  Author: Ariel Levkovich <lariel at nvidia.com>
  Date:   2021-04-19 (Mon, 19 Apr 2021)

  Changed paths:
    M lib/netdev-offload-tc.c

  Log Message:
  -----------
  netdev-offload-tc: Add support for ct_state flag rel.

Signed-off-by: Ariel Levkovich <lariel at nvidia.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner at gmail.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 3311ca0d449e02802645d3d459a59ab19e0aa7aa
      https://github.com/openvswitch/ovs/commit/3311ca0d449e02802645d3d459a59ab19e0aa7aa
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M AUTHORS.rst

  Log Message:
  -----------
  AUTHORS: Add Ariel Levkovich.

Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 4a6be85c8d2e9c04e2a1d70370f5f814c894ee25
      https://github.com/openvswitch/ovs/commit/4a6be85c8d2e9c04e2a1d70370f5f814c894ee25
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M lib/odp-util.c

  Log Message:
  -----------
  odp-util: Fix use of uninitialized erspan metadata.

'struct erspan_metadata' contains union with fields of different
sizes, hence not all the memory initiliazed.  This memory goes
to syscalls and also used to compare ukeys with memcmp which may
cause unexpected behavior.

 Thread 15 revalidator13:
 Conditional jump or move depends on uninitialised value(s)
    at 0x4C377B6: bcmp (vg_replace_strmem.c:1111)
    by 0x43F844: ofpbuf_equal (ofpbuf.h:273)
    by 0x43F844: revalidate_ukey__ (ofproto-dpif-upcall.c:2227)
    by 0x43F9C9: revalidate_ukey (ofproto-dpif-upcall.c:2294)
    by 0x4425C2: revalidate.isra.33 (ofproto-dpif-upcall.c:2734)
    by 0x4434B8: udpif_revalidator (ofproto-dpif-upcall.c:943)
    by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
    by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
    by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
  Uninitialised value was created by a stack allocation
    at 0x4B1CE0: tun_key_to_attr (odp-util.c:3129)

Fixes: 98514eea21f4 ("erspan: add kernel datapath support")
Acked-by: William Tu <u9012063 at gmail.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: f9d30390392c120ed7e57a84bf00b3935f35f39d
      https://github.com/openvswitch/ovs/commit/f9d30390392c120ed7e57a84bf00b3935f35f39d
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M lib/dpif.c
    M lib/dpif.h

  Log Message:
  -----------
  dpif: Fix use of uninitialized execute hash.

'dpif_execute_helper_cb' doesn't initilalize the 'hash' field that
may be passed down to datapath and might cause execution of a different
set of actions, e.g. on recirculation.

 Thread 6 handler27:
 Conditional jump or move depends on uninitialised value(s)
    at 0x53A2C2: dpif_netlink_encode_execute (dpif-netlink.c:1841)
    by 0x53A2C2: dpif_netlink_operate__ (dpif-netlink.c:1919)
    by 0x53A82D: dpif_netlink_operate_chunks (dpif-netlink.c:2238)
    by 0x53A82D: dpif_netlink_operate (dpif-netlink.c:2297)
    by 0x48135F: dpif_operate (dpif.c:1366)
    by 0x481923: dpif_execute.part.24 (dpif.c:1320)
    by 0x481C46: dpif_execute (dpif.c:1312)
    by 0x481C46: dpif_execute_helper_cb (dpif.c:1243)
    by 0x4AE943: odp_execute_actions (odp-execute.c:865)
    by 0x47F272: dpif_execute_with_help (dpif.c:1296)
    by 0x4812FF: dpif_operate (dpif.c:1422)
    by 0x442226: handle_upcalls (ofproto-dpif-upcall.c:1617)
    by 0x442226: recv_upcalls.isra.36 (ofproto-dpif-upcall.c:855)
    by 0x442351: udpif_upcall_handler (ofproto-dpif-upcall.c:755)
    by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
    by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
    by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
  Uninitialised value was created by a stack allocation
    at 0x481966: dpif_execute_helper_cb (dpif.c:1159)

Additionally added a missing comment to the 'struct dpif_execute'.

Fixes: 0442bfb11d6c ("ofproto-dpif-upcall: Echo HASH attribute back to datapath.")
Acked-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: d90b4f29283b2ebfaad8248f6ba5ecccf7a154d6
      https://github.com/openvswitch/ovs/commit/d90b4f29283b2ebfaad8248f6ba5ecccf7a154d6
  Author: Michal Kazior <michal at plume.com>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M lib/if-notifier.c
    M lib/netdev-linux.c
    M lib/route-table.c
    M lib/rtnetlink.c
    M lib/rtnetlink.h

  Log Message:
  -----------
  rtnetlink: ignore IFLA_WIRELESS events.

Some older wireless drivers - ones relying on the
old and long deprecated wireless extension ioctl
system - can generate quite a bit of IFLA_WIRELESS
events depending on their configuration and
runtime conditions. These are delivered as
RTNLGRP_LINK via RTM_NEWLINK messages.

These tend to be relatively easily identifiable
because they report the change mask being 0. This
isn't guaranteed but in practice it shouldn't be a
problem. None of the wireless events that I ever
observed actually carry any unique information
about netdev states that ovs-vswitchd is
interested in. Hence ignoring these shouldn't
cause any problems.

These events can be responsible for a significant
CPU churn as ovs-vswitchd attempts to do plenty of
work for each and every netlink message regardless
of what that message carries. On low-end devices
such as consumer-grade routers these can lead to a
lot of CPU cycles being wasted, adding up to heat
output and reducing performance.

It could be argued that wireless drivers in
question should be fixed, but that isn't exactly a
trivial thing to do. Patching ovs seems far more
viable while still making sense.

Signed-off-by: Michal Kazior <michal at plume.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: fd1114e9671b14f4bd14bf605641ae1785107120
      https://github.com/openvswitch/ovs/commit/fd1114e9671b14f4bd14bf605641ae1785107120
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M AUTHORS.rst

  Log Message:
  -----------
  AUTHORS: Add Michal Kazior.

Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


  Commit: 5dce24d04de7e1210bf6215955bcc17489010ada
      https://github.com/openvswitch/ovs/commit/5dce24d04de7e1210bf6215955bcc17489010ada
  Author: Mark Gray <mark.d.gray at redhat.com>
  Date:   2021-04-20 (Tue, 20 Apr 2021)

  Changed paths:
    M tests/system-ipsec.at

  Log Message:
  -----------
  ipsec: Fix race in system tests.

This patch fixes an issue where, depending on timing fluctuations,
each node has not fully loaded all connections before the other
node begins to establish a connection. In this failure case, the
"ovs-monitor-ipsec" instance on the "left" node may `ipsec auto --start`
a connection which then gets rejected by the "right" side. Almost,
simulaneously, the "right" side may initiate a connection that gets
rejected by the "left" side. This can happen as, for all tunnels except
for GRE, each node has two connections (an "in" connection and an "out"
connection) that get added one after the other. If the "in" connection
"starts" on both sides, the "out" connection from the other node
may not be available causing the connection to fail. At this point,
"Libreswan" will wait to retry the connection. In the interim, the
OVS system test times out. This race manifests itself more frequently
in a virtualized environment.

This patch resolves this issue by waiting for the "left" node to load
all connections before starting the "right" side. This will cause
the "left" side to fail to establish a connection with the "right"
side (as the "right" side connections have not been loaded) but will
cause the "right" side to succeed to establish a connection as all
connections will have been loaded on the "left" side.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-April/381857.html
Fixes: 8fc62df8b135 ("ipsec: Introduce IPsec system tests for Libreswan.")
Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
Tested-by: Flavio Leitner <fbl at sysclose.org>
Acked-by: Flavio Leitner <fbl at sysclose.org>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/44ea24427e8b...5dce24d04de7


More information about the git mailing list