diff --git a/SOURCES/libteam-teamd-do-no-remove-the-ports-on-shutdown-with-N.patch b/SOURCES/libteam-teamd-do-no-remove-the-ports-on-shutdown-with-N.patch new file mode 100644 index 0000000..8bf65e7 --- /dev/null +++ b/SOURCES/libteam-teamd-do-no-remove-the-ports-on-shutdown-with-N.patch @@ -0,0 +1,46 @@ +From dbb2cfca35d2cd15125eb84e8f3940f8cc3ea860 Mon Sep 17 00:00:00 2001 +From: Lubomir Rintel +Date: Mon, 10 Oct 2022 18:33:53 +0200 +Subject: [PATCH] teamd: do no remove the ports on shutdown with -N + +With -N, teamd currently leaves the team device in place on shutdown, +as it's supposed to, but it removes all the ports. This severely limits +usefullness of the option, because it's still impossible to replace +the daemon with another one without disrupting connectivity. + +One use case where this is important is the handover from initrd to real +root, when a team device was used to provide connectivity to a network +root filesystem: + +Systemd's isolation of switch-root.target stops NetworkManager.service and +then terminates its kids, including teamd. The real NetworkManager.service +would eventually catch up and restart it, but there's a short period when +team ports are removed which is not great if we're booting off that device. +Also, it may be that ports come up in different order, causing team to get +a different MAC address, which will invalidate the DHCP lease we got +beforehands and screwing up L3 addressing. + +Let's not flush the ports when -N is used. + +Suggested-by: Jiri Pirko +Signed-off-by: Lubomir Rintel +Signed-off-by: Jiri Pirko +--- + teamd/teamd_per_port.c | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c +index d429753..9689df4 100644 +--- a/teamd/teamd_per_port.c ++++ b/teamd/teamd_per_port.c +@@ -217,7 +217,6 @@ static void port_obj_remove(struct teamd_context *ctx, + struct teamd_port *tdport = _port(port_obj); + + teamd_event_port_removed(ctx, tdport); +- teamd_port_remove(ctx, tdport); + port_obj_destroy(ctx, port_obj); + port_obj_free(port_obj); + } +-- +2.31.1 + diff --git a/SOURCES/libteam-teamd-stop-iterating-callbacks-when-a-loop-restart-i.patch b/SOURCES/libteam-teamd-stop-iterating-callbacks-when-a-loop-restart-i.patch new file mode 100644 index 0000000..064c2f1 --- /dev/null +++ b/SOURCES/libteam-teamd-stop-iterating-callbacks-when-a-loop-restart-i.patch @@ -0,0 +1,105 @@ +From ffc6a52bd285a476b547312012078af69220574b Mon Sep 17 00:00:00 2001 +From: Lubomir Rintel +Date: Mon, 10 Oct 2022 18:37:31 +0200 +Subject: [PATCH] teamd: stop iterating callbacks when a loop restart is + requested + +A crash was observed: + + Added loop callback: dbus_watch, 0x560d279e4530 + Added loop callback: dbus_watch, 0x560d279e4580 + ... + Removed loop callback: dbus_watch, 0x560d279e4580 + Removed loop callback: dbus_watch, 0x560d279e4530 + Aug 31 11:54:11 holaprdel kernel: traps: teamd[557] general protection fault ip:560d26469a55 sp:7ffd43ca9650 error:0 in teamd[560d26463000+16000] + +Traceback (from a different run than above): + + Core was generated by `/usr/bin/teamd -o -n -U -D -N -t team0 -gg'. + Program terminated with signal SIGSEGV, Segmentation fault. + #0 0x00005600ac090a55 in teamd_run_loop_do_callbacks (ctx=0x5600ad9bac70, fds=0x7fff40861250, lcb_list=0x5600ad9bad58) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:321 + 321 list_for_each_node_entry_safe(lcb, tmp, lcb_list, list) { + (gdb) bt + #0 0x00005600ac090a55 in teamd_run_loop_do_callbacks (ctx=0x5600ad9bac70, fds=0x7fff40861250, lcb_list=0x5600ad9bad58) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:321 + #1 teamd_run_loop_run (ctx=0x5600ad9bac70) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:415 + #2 0x00005600ac08d8cb in teamd_start (p_ret=, ctx=) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:1557 + #3 main (argc=, argv=) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:1876 + (gdb) print *lcb + Cannot access memory at address 0x9dd29944fb7e97fc + (gdb) print *tmp + Cannot access memory at address 0x9dd29944fb7e9804 + (gdb) + +What has happened is that libdbus called the remove_watch callback twice +in a single go, causing two callbacks being destroyed in one iteration +of teamd_run_loop_do_callbacks()'s list_for_each_node_entry_safe(). + +This basically turns the _safe() variant of the macro unhelpful, as tmp +points to stale data anyway. + +Let's use the unsafe variant then, and terminate the loop once +teamd_loop_callback_del() asks for main loop's attention via +teamd_run_loop_restart(). If there are other callbacks pending an action, +they will get their turn in the next main loop iteration. + +Signed-off-by: Lubomir Rintel +Signed-off-by: Jiri Pirko +--- + teamd/teamd.c | 26 ++++++++++++++++++++++++-- + 1 file changed, 24 insertions(+), 2 deletions(-) + +diff --git a/teamd/teamd.c b/teamd/teamd.c +index b310140..a89b702 100644 +--- a/teamd/teamd.c ++++ b/teamd/teamd.c +@@ -309,16 +309,28 @@ static void teamd_run_loop_set_fds(struct list_item *lcb_list, + } + } + ++static int teamd_check_ctrl(struct teamd_context *ctx) ++{ ++ int ctrl_fd = ctx->run_loop.ctrl_pipe_r; ++ struct timeval tv; ++ fd_set rfds; ++ ++ FD_ZERO(&rfds); ++ FD_SET(ctrl_fd, &rfds); ++ tv.tv_sec = tv.tv_usec = 0; ++ ++ return select(ctrl_fd + 1, &rfds, NULL, NULL, &tv); ++} ++ + static int teamd_run_loop_do_callbacks(struct list_item *lcb_list, fd_set *fds, + struct teamd_context *ctx) + { + struct teamd_loop_callback *lcb; +- struct teamd_loop_callback *tmp; + int i; + int events; + int err; + +- list_for_each_node_entry_safe(lcb, tmp, lcb_list, list) { ++ list_for_each_node_entry(lcb, lcb_list, list) { + for (i = 0; i < 3; i++) { + if (!(lcb->fd_event & (1 << i))) + continue; +@@ -339,6 +351,16 @@ static int teamd_run_loop_do_callbacks(struct list_item *lcb_list, fd_set *fds, + teamd_log_dbg(ctx, "Failed loop callback: %s, %p", + lcb->name, lcb->priv); + } ++ ++ /* ++ * If there's a control byte ready, it's possible that ++ * one or more entries have been removed from the ++ * callback list and restart has been requested. In any ++ * case, let the main loop deal with it first so that ++ * we know we're safe to proceed. ++ */ ++ if (teamd_check_ctrl(ctx)) ++ return 0; + } + } + return 0; +-- +2.31.1 + diff --git a/SOURCES/libteamdctl-validate-the-bus-name-before-using-it.patch b/SOURCES/libteamdctl-validate-the-bus-name-before-using-it.patch new file mode 100644 index 0000000..b095f98 --- /dev/null +++ b/SOURCES/libteamdctl-validate-the-bus-name-before-using-it.patch @@ -0,0 +1,61 @@ +From 3bbce8a171deab6cd3d7d57d128bc2dbaea451f0 Mon Sep 17 00:00:00 2001 +Message-Id: <3bbce8a171deab6cd3d7d57d128bc2dbaea451f0.1664556124.git.lucien.xin@gmail.com> +From: Xin Long +Date: Fri, 15 Apr 2022 11:41:39 -0400 +Subject: [PATCH] libteamdctl: validate the bus name before using it + +Using bus name without validating it will cause core dump generated, +and it can be reproduced by: + + # ip link add dummy0.1 type dummy + # teamdctl dummy0.1 state dump + + This is normally a bug in some application using the D-Bus library. + + D-Bus not built with -rdynamic so unable to print a backtrace + Aborted (core dumped) + +Doing this many times can even create too many core files, customers +may complain about it. + +This is triggered when calling cli_method_call("ConfigDump") in +cli_init(), so fix it by returning err in cli->init/cli_dbus_init() +if the bus name fails to validate. + +Note this is safe, as with dbus, we can't use invalid dbus name to +create the team dev either. + +Fixes: d8163e34c25c ("libteamdctl: do test method call instead or Introspect call") +Reported-by: Uday Patel +Signed-off-by: Xin Long +Signed-off-by: Jiri Pirko +--- + libteamdctl/cli_dbus.c | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/libteamdctl/cli_dbus.c b/libteamdctl/cli_dbus.c +index dfef5c4..242ef86 100644 +--- a/libteamdctl/cli_dbus.c ++++ b/libteamdctl/cli_dbus.c +@@ -183,12 +183,17 @@ static int cli_dbus_init(struct teamdctl *tdc, const char *team_name, void *priv + if (ret == -1) + return -errno; + ++ err = -EINVAL; + dbus_error_init(&error); ++ if (!dbus_validate_bus_name(cli_dbus->service_name, &error)) { ++ err(tdc, "dbus: Could not validate bus name: %s - %s", ++ error.name, error.message); ++ goto free_service_name; ++ } + cli_dbus->conn = dbus_bus_get(DBUS_BUS_SYSTEM, &error); + if (!cli_dbus->conn) { + err(tdc, "dbus: Could not acquire the system bus: %s - %s", + error.name, error.message); +- err = -EINVAL; + goto free_service_name; + } + err = 0; +-- +2.27.0 + diff --git a/SPECS/libteam.spec b/SPECS/libteam.spec index ccbd5bb..500aacc 100644 --- a/SPECS/libteam.spec +++ b/SPECS/libteam.spec @@ -1,12 +1,15 @@ Name: libteam Version: 1.31 -Release: 2%{?dist} +Release: 4%{?dist} Summary: Library for controlling team network device Group: System Environment/Libraries License: LGPLv2+ URL: http://www.libteam.org Source: http://www.libteam.org/files/libteam-%{version}.tar.gz Patch1: libteam-Revert-teamd-Disregard-current-state-when-considerin.patch +Patch2: libteamdctl-validate-the-bus-name-before-using-it.patch +Patch3: libteam-teamd-do-no-remove-the-ports-on-shutdown-with-N.patch +Patch4: libteam-teamd-stop-iterating-callbacks-when-a-loop-restart-i.patch BuildRequires: jansson-devel BuildRequires: libdaemon-devel BuildRequires: libnl3-devel @@ -165,6 +168,11 @@ cd binding/python %{_sysconfdir}/sysconfig/network-scripts/ifdown-TeamPort %changelog +* Mon Dec 05 2022 Xin Long - 1.31-4 +- teamd: do no remove the ports on shutdown with -N [2148856] +- teamd: stop iterating callbacks when a loop restart is requested [2148855] +* Fri Sep 30 2022 Xin Long - 1.31-3 +- libteamdctl: validate the bus name before using it [2065227] * Tue Sep 01 2020 Xin Long - 1.31-2 - Revert "teamd: Disregard current state when considering port enablement" [1874001] * Thu Jul 30 2020 Xin Long - 1.31-1