Package: ifupdown Version: 0.7.47.2 Severity: important Tags: patch User: [email protected] Usertags: origin-ubuntu utopic ubuntu-patch
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Hello Maintainers, In Ubuntu, the attached patch was applied in a hotfix repository: * Removed buggy fine-grained lock and introduced big-lock (LP: #1337873) Description: It was brought to my attention (by others) that ifupdown runs into race conditions on some specific cases. [Impact] When trying to deploy many servers at once (higher chances of happening) or from time-to-time, like any other intermittent race-condition. Interfaces are not brought up like they should and this has a big impact for servers that cannot rely on network start scripts. The problem is caused by a race condition when init(upstart) starts up network interfaces in parallel. [Test Case] Use attached script to reproduce the error (it might take some hours, in a single virtual machine, for the error to occur). (example 1) *** sequence to trigger race-condition *** (a) ifup eth0 (b) ifup -a for eth0 - ----------------------------------------------------------------- 1-1. Lock ifstate.lock file. 1-1. Wait for locking ifstate.lock file. 1-2. Read ifstate file to check the target NIC. 1-3. close(=release) ifstate.lock file. 1-4. Judge that the target NIC isn't processed. 1-2. Read ifstate file to check the target NIC. 1-3. close(=release) ifstate.lock file. 1-4. Judge that the target NIC isn't processed. 2. Lock and update ifstate file. Release the lock. 2. Lock and update ifstate file. Release the lock. (example 2) Bonding device using eth0. ifenslave for eth0 is also executed in parallel, eth0 remains down. *** sequence to trigger race-condition *** (a) ifenslave of eth0 (b) ifenslave of eth0 - ------------------------------------------------------------------ 3. Execute ifenslave of eth0. 3. Execute ifenslave of eth0. 4. Link down the target NIC. 5. Write NIC id to /sys/class/net/bond0/bonding /slaves then NIC gets up 4. Link down the target NIC. 5. Fails to write NIC id to /sys/class/net/bond0/bonding/ slaves it is already written. (example 3) bonding is not set to active-backup as defined in config file: When the init(upstart) executes "if-pre-up.d/ifenslave" script and "if-pre-up.d/vlan" script for bond0 device in parallel, the "if-pre-up.d/ifenslave" script fails to change the bonding mode with a error message, "bonding: unable to update mode of bond0 because interface is up.". *** sequence to trigger race-condition *** (a)ifup bond0 (b)ifup -a - ----------------------------------------------------------------------- 1. Update statefile about bond0. 1. Does nothing about bond0 because statefile is already updated about it. 2. ifenslave::setup_master() sysfs_change_down mode 1 and link down bond0. 2. Link up bond0 by the vlan script on the processing for linking up bond0.201(*1). 3. "echo 1 > .../mode" fails. [ /etc/network/if-pre-up.d/vlan ] 46 if [ -n "$IF_VLAN_RAW_DEVICE" ] && [ ! -d /sys/class/net/$IFACE ]; then 47 if [ ! -x /sbin/vconfig ]; then 48 exit 0 49 fi 50 if ! ip link show dev "$IF_VLAN_RAW_DEVICE" > /dev/null; then 51 echo "$IF_VLAN_RAW_DEVICE does not exist, unable to create $IFACE" 52 exit 1 53 fi 54 ip link set up dev $IF_VLAN_RAW_DEVICE <-- (*1). 55 vconfig add $IF_VLAN_RAW_DEVICE $VLANID 56 fi [Regression Potential] * Not fix an already existent race condition. [Other Info] Example: [ /etc/network/interfaces ] auto lo iface lo inet loopback auto eth0 iface eth0 inet manual bond-master bond0 auto eth1 iface eth1 inet manual bond-master bond0 auto bond0 iface bond0 inet dhcp bond-slaves eth0 eth1 hwaddress 11:22:33:44:55:66 bond-primary eth0 bond-mode 1 bond-miimon 100 bond-updelay 200 bond-downdelay 200 auto bond0.201 iface bond0.201 inet dhcp hwaddress 11:22:33:44:55:66 vlan-raw-device bond0 .... auto bond0.205 iface bond0.205 inet dhcp hwaddress 11:22:33:44:55:66 vlan-raw-device bond0 Thanks for considering this patch. Let me know if there is anything I can do to help you with this. Regards Rafael Tinoco - -- System Information: Debian Release: jessie/sid APT prefers trusty-updates APT policy: (500, 'trusty-updates'), (500, 'trusty-security'), (500, 'trusty'), (100, 'trusty-backports') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 3.13.0-30-generic (SMP w/8 CPU cores) Locale: LANG=en_US.UTF-8, LC_CTYPE=UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF-8) Shell: /bin/sh linked to /bin/dash -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4 iQEcBAEBCgAGBQJTtuWZAAoJEAynk4KHaD/AKM4H/j/8IjttNkJqbc65My1xhpEE /O+2jwzarEjtzg4d8IzipK52ebWlzfiJFsIAKIOjJzjgyqsrbZ14PbK3NcN3QMCi vq4JAW81dhnKeYO1w0xtOvM/g0wYlSqy9dPrPlriX1pg7icapYo5xrG78y/WVCXL vKQoQgWj/dIHQSRBTgjPIPTQohXu0kNYfOzdV7c/D/4HbqpCxnQd3nx4k6cGOZem ucW7S0dPaxwnIs0oPML8dOGPvF7ei0k/defhrjFTBjuajmpanfLvlc42kt4SNhRa MNVCTA4q7viy3MPbZVUz0AWC7k23YguTq+5VKTqDWyNqu2VStgw52RM9eARYpmQ= =xCRD -----END PGP SIGNATURE-----
diff -Nru ifupdown-0.7.47.2ubuntu4.1/debian/changelog ifupdown-0.7.47.2ubuntu4.2~lp1337873/debian/changelog diff -Nru ifupdown-0.7.47.2ubuntu4.1/debian/control ifupdown-0.7.47.2ubuntu4.2~lp1337873/debian/control --- ifupdown-0.7.47.2ubuntu4.1/debian/control 2013-12-02 20:55:40.000000000 -0200 +++ ifupdown-0.7.47.2ubuntu4.2~lp1337873/debian/control 2014-07-04 14:29:27.000000000 -0300 @@ -1,8 +1,7 @@ Source: ifupdown Section: admin Priority: important -Maintainer: Ubuntu Developers <[email protected]> -XSBC-Original-Maintainer: Andrew Shadura <[email protected]> +Maintainer: Andrew Shadura <[email protected]> Standards-Version: 3.9.4 Build-Depends: debhelper (>= 9.20120410~) Vcs-Hg: http://anonscm.debian.org/hg/collab-maint/ifupdown/ diff -Nru ifupdown-0.7.47.2ubuntu4.1/main.c ifupdown-0.7.47.2ubuntu4.2~lp1337873/main.c --- ifupdown-0.7.47.2ubuntu4.1/main.c 2014-03-21 16:27:36.000000000 -0300 +++ ifupdown-0.7.47.2ubuntu4.2~lp1337873/main.c 2014-07-04 14:12:47.000000000 -0300 @@ -126,46 +126,37 @@ exit(0); } -FILE * lock_state(const char * argv0) { - FILE *lock_fp; - lock_fp = fopen(lockfile, no_act ? "r" : "a+"); - if (lock_fp == NULL) { - if (!no_act) { - fprintf(stderr, "%s: failed to open lockfile %s: %s\n", argv0, lockfile, strerror(errno)); - exit(1); - } else { - return NULL; - } - } - - int flags; - - if ((flags = fcntl(fileno(lock_fp), F_GETFD)) < 0 || fcntl(fileno(lock_fp), F_SETFD, flags | FD_CLOEXEC) < 0) { - fprintf(stderr, "%s: failed to set FD_CLOEXEC on lockfile %s: %s\n", argv0, lockfile, strerror(errno)); - exit(1); - } +int fakespin(void) +{ + static int fd = -1; + struct flock fl; - if (lock_fd(fileno(lock_fp)) < 0) { - if (!no_act) { - fprintf(stderr, "%s: failed to lock lockfile %s: %s\n", argv0, lockfile, strerror(errno)); - exit(1); + fl.l_type = F_WRLCK; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 1; + + if (fd == -1) + fd = open(lockfile, O_WRONLY|O_CREAT, 0666); + + if (fd != -1) + if((fcntl(fd, F_SETLK, &fl)) == -1) + { + usleep(250000); + return 1; } - } - return lock_fp; + return 0; } static const char *read_state(const char *argv0, const char *iface) { char *ret = NULL; - FILE *lock_fp; FILE *state_fp; char buf[80]; char *p; - lock_fp = lock_state(argv0); - state_fp = fopen(statefile, no_act ? "r" : "a+"); if (state_fp == NULL) { if (!no_act) { @@ -211,24 +202,16 @@ state_fp = NULL; } - if (lock_fp != NULL) { - fclose(lock_fp); - lock_fp = NULL; - } - return ret; } static void read_all_state(const char *argv0, char ***ifaces, int *n_ifaces) { int i; - FILE *lock_fp; FILE *state_fp; char buf[80]; char *p; - lock_fp = lock_state(argv0); - state_fp = fopen(statefile, no_act ? "r" : "a+"); if (state_fp == NULL) { if (!no_act) { @@ -279,24 +262,16 @@ fclose(state_fp); state_fp = NULL; } - - if (lock_fp != NULL) { - fclose(lock_fp); - lock_fp = NULL; - } } static void update_state(const char *argv0, const char *iface, const char *state) { FILE *tmp_fp; - FILE *lock_fp; FILE *state_fp; char buf[80]; char *p; - lock_fp = lock_state(argv0); - state_fp = fopen(statefile, no_act ? "r" : "a+"); if (state_fp == NULL) { if (!no_act) { @@ -370,11 +345,6 @@ fclose(state_fp); state_fp = NULL; } - - if (lock_fp != NULL) { - fclose(lock_fp); - lock_fp = NULL; - } } static int lock_fd(int fd) @@ -632,6 +602,9 @@ usage(argv[0]); } + while(fakespin()) + continue; + defn = read_interfaces(interfaces); if (!defn) { fprintf(stderr, "%s: couldn't read interfaces file \"%s\"\n", argv[0], interfaces);
