Created attachment 1201204 [details] Python script reproducing the issue Description of problem: When I try to call Reapply of a bond slave ethernet connection NetworkManager crashes with a segfault (based on log in journalctl). This is using the interface exposed over D-Bus, using python. Version-Release number of selected component (if applicable): NetworkManager-1.4.0-4.fc26.x86_64 How reproducible: always Steps to Reproduce: 1. add bond connection 2. add slave ethernet connection 3. activate bond connection 4. activate slave connection 5. call Reapply on the device that the slave connection is applied on Actual results: Crash with segfault. Expected results: Nothing happens. Maybe an error of an invalid or "empty" operation. Additional info: python reproducer attached, example use: ./nm_reproducer.py ens9 52:54:00:38:c4:24
Created attachment 1201521 [details] [PATCH] device: fix crash reapplying connection to slave devices
(In reply to Beniamino Galvani from comment #1) > Created attachment 1201521 [details] > [PATCH] device: fix crash reapplying connection to slave devices Maybe method_old = s_ip4_old ? nm_setting_ip_config_get_method (s_ip4_old) : NM_SETTING_IP4_CONFIG_METHOD_DISABLED; should be method_old = s_ip4_old ? nm_setting_ip_config_get_method (s_ip4_old) : NULL; and use nm_streq0() below. A s_ip4_old==NULL means that it is a slave. A slave doesn't really have ip4.method=DISABLED, it has no IPv4 altogether. Thus, I'd rather compare the NULL there directly, instead of coercing the method to DISABLED. Of course, in reality it doesn't make a difference, because reapply does not handle the change slave to non-slave or vice versa. Thus, old and new are always either both NULL, or both non-NULL. But still seems more logical to me.
(In reply to Thomas Haller from comment #2) > A s_ip4_old==NULL means that it is a slave. A slave doesn't really have > ip4.method=DISABLED, it has no IPv4 altogether. Thus, I'd rather compare the > NULL there directly, instead of coercing the method to DISABLED. > > > Of course, in reality it doesn't make a difference, because reapply does not > handle the change slave to non-slave or vice versa. Thus, old and new are > always either both NULL, or both non-NULL. But still seems more logical to > me. I tried to make it consistent with function nm_utils_get_ip_config_method() which returns DISABLED/IGNORE for slaves (we can't use that function because it needs a connection). But as you said, there is really no difference in this case, both ways look fine to me.
(In reply to Beniamino Galvani from comment #3) > (In reply to Thomas Haller from comment #2) ok. lgtm then.
lgtm
Patch applied to master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=8f92ead6e2a9082d5aa4a885680308b4516d70ae and nm-1-4: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=nm-1-4&id=dbb67694cbc51eb639bd5d7d60d1455e4f5a449a
This bug appears to have been reported against 'rawhide' during the Fedora 26 development cycle. Changing version to '26'.
rawhide currently has NM 1.6.0, which has the fix. Closing.