Thanks a lot for the report and the fix!
Our usual practice is to put the test and the fix into the same patch,
so I squashed these together. I applied the squashed patches to master
and backported as far as branch-2.5. I see that the bug probably
existed before that, but there was a patch reject at that point and I
didn't see too many value in going farther.
Thanks,
Ben.
On Tue, May 01, 2018 at 07:07:35PM -0400, Flavio Fernandes wrote:
> Greetings!
>
> I was lucky enough to spend some time having fun with OVS recently and
> encountered
> a bug that may be worth sharing with you.
>
> The code path in the error case when bridge's del-port is attempted for the
> parent (instead of the fake-bridge) segfaults. Here are some simple steps for
> reproducing this issue:
>
> ./boot.sh ; ./configure ; make -j4 ; make sandbox
>
> PARENT_BRIDGE=br0 ; FAKE_BRIDGE=br0c ; VLAN_TAG=666
> ovs-vsctl add-br ${PARENT_BRIDGE}
> ovs-vsctl add-br $FAKE_BRIDGE $PARENT_BRIDGE $VLAN_TAG
>
> # Add a port to parent bridge, which happens to have same tag as the
> fake_bridge
> # Note: The port could be have been added directly to the fake bridge too, of
> course.
> # The end result of the add-port is the same.
> ovs-vsctl add-port $PARENT_BRIDGE p1 -- set port p1 tag=${VLAN_TAG}
> # ovs-vsctl add-port $FAKE_BRIDGE p1
>
> # removing p1 will make a segfault
> ovs-vsctl del-port $PARENT_BRIDGE p1 ; # sad panda moment
>
> # Here are 3 ways of working around this segfault
>
> # workaround 1: remove tag before removing port from parent
> ovs-vsctl remove port p1 tag $VLAN_TAG && \
> ovs-vsctl del-port $PARENT_BRIDGE p1 && echo ok
> # workaround 2: remove port as if it belongs to fake bridge
> ovs-vsctl del-port $FAKE_BRIDGE p1 && echo ok
> # workaround 3: remove port w/out specifying a bridge
> ovs-vsctl del-port p1 && echo ok
>
>
> This issue appears to exist since commit 7c79588e , which dates back to
> Feb/2010.
> I see it in OVS 2.3.2.
>
> -- flaviof
>
>
>
> More details about the segfault:
>
> |main: Ubuntu ~/ovs.git/_ffbuilddir/utilities/sandbox on devel
> $ sudo ovs-vsctl show
> fa096c6f-8f5f-49ae-92b4-e94ce58aceec
> Bridge "br0"
> Port "br0"
> Interface "br0"
> type: internal
> Port "p1"
> tag: 666
> Interface "p1"
> Port "br0c"
> tag: 666
> Interface "br0c"
> type: internal
> |main: Ubuntu ~/ovs.git/_ffbuilddir/utilities/sandbox on devel
> $ gdb /home/ff/ovs.git/_ffbuilddir/utilities/ovs-vsctl
> ...
> Reading symbols from /home/ff/ovs.git/_ffbuilddir/utilities/ovs-vsctl...done.
> (gdb) run del-port br0 p1
> Starting program: /home/ff/ovs.git/_ffbuilddir/utilities/ovs-vsctl del-port
> br0 p1
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000040a945 in cmd_del_port (ctx=0x7fffffffc5d0) at
> ../utilities/ovs-vsctl.c:1750
> 1750 ctl_fatal("bridge %s does not have a port %s
> (although "
> (gdb) bt
> #0 0x000000000040a945 in cmd_del_port (ctx=0x7fffffffc5d0) at
> ../utilities/ovs-vsctl.c:1750
> #1 0x00000000004075dd in do_vsctl (args=0xc8f7f0
> "/home/ff/ovs.git/_ffbuilddir/utilities/ovs-vsctl del-port br0 p1",
> commands=0xc8fd50, n_commands=1, idl=0xc8fdb0)
> at ../utilities/ovs-vsctl.c:2623
> #2 0x0000000000405e7e in main (argc=4, argv=0x7fffffffc868) at
> ../utilities/ovs-vsctl.c:184
> (gdb) list
> 1745 struct vsctl_bridge *bridge;
> 1746
> 1747 bridge = find_bridge(vsctl_ctx, ctx->argv[1], true);
> 1748 if (port->bridge != bridge) {
> 1749 if (port->bridge->parent == bridge) {
> 1750 ctl_fatal("bridge %s does not have a port %s
> (although "
> 1751 "its parent bridge %s does)",
> 1752 ctx->argv[1], ctx->argv[2],
> 1753 bridge->parent->name);
> 1754 } else {
> (gdb) p ctx->argv[1]
> $1 = 0x7fffffffcba5 "br0"
> (gdb) p ctx->argv[2]
> $2 = 0x7fffffffcba9 "p1"
> (gdb) p port->bridge->name
> $3 = 0xc9a290 "br0c"
> (gdb) p bridge
> $4 = (struct vsctl_bridge *) 0xcc9d60
> (gdb) p bridge->parent
> $5 = (struct vsctl_bridge *) 0x0
> (gdb) p port->bridge
> $6 = (struct vsctl_bridge *) 0xccb440
> (gdb) p bridge->name
> $7 = 0xcc8da0 "br0"
> (gdb)
>
>
> Flavio Fernandes (2):
> ovs-vsctl.at: deleting a port from fake bridge
> ovs-vsctl: Fix segfault when attempting to del-port from parent
> bridge.
>
> tests/ovs-vsctl.at | 17 +++++++++++++++++
> utilities/ovs-vsctl.c | 4 ++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
