wait-for-root fails to wait for plain /dev/sdaX partitions.

[Tetsuo Handa]

SRU Justification:
[Impact]
  * Boot failures can occur with the wait-for-root utility in P/Q/R due to a race condition.
  * Because of this issue unattended reboots and boots can randomly fail.
  * The original bug was submitted against Precise LTS.

[Test Case]
 * Reboot machine and look for "ALERT! /dev/sda1 does not exist. Dropping to a shell!". Entering exit from prompt should boot system normally.
 * We expect that continuous reboots should allow for the machine to boot normally without this alert.

[Regression Potential]
 * This patch has already been uploaded into Saucy, and tested.

--

Moving the discussion from http://www.spinics.net/lists/hotplug/msg05769.html
to launchpad, for I think that this bug needs to be handled in initramfs-tools
package rather than in udev package.

----------

I'm experiencing random boot failures with wait-for-root utility in Ubuntu
12.04 ( ubuntu-12.04-server-amd64.iso ) on a HP ProLiant DL360p Gen8 server.

For example, wait-for-root waited for only 0.13 seconds before giving up at

  FSTYPE=$(wait-for-root "${ROOT}" ${ROOTDELAY:-30})

line in scripts/local in the initramfs, and immediately enters into

  panic "ALERT! ${ROOT} does not exist. Dropping to a shell!"

line.

This is a race condition and manually entering "exit" from the panic prompt
boots the system normally. This is a critical bug for this environment because
it will randomly fail to perform unattended reboot (e.g. automatic reboot after
saving kdump).

----------

I examined main() in wait-for-root using debug fprintf() and it turned out that
udev_monitor_receive_device() is sometimes immediately returning NULL (although
wait-for-root is using blocking socket).

I examined udev_monitor_receive_device() in libudev.so.0 using debug fprintf()
and it turned out that recvmsg() in udev_monitor_receive_device() (which is in
libudev-monitor.c in udev package) is returning ENOBUFS error before recvmsg()
returns information of the root partition.

The wait-for-root utility in initramfs-tools package is not expecting recvmsg()
to return ENOBUFS error. But since ENOBUFS is an inevitable error, I think that
wait-for-root (i.e. the caller of udev_monitor_receive_device()) should handle
this error.

[Tetsuo Handa (9-launchpad-i-love-sakura-ne-jp)]

I confirmed that below patch fixes my problem.

---------- patch start ----------
--- a/src/wait-for-root.c
+++ b/src/wait-for-root.c
@@ -88,7 +88,9 @@ main (int argc,
  /* When the device doesn't exist yet, or is still being processed
   * by udev, use the monitor socket to wait it to be done.
   */
- while ((udev_device = udev_monitor_receive_device (udev_monitor)) != NULL) {
+ while (1) {
+ while ((udev_device = udev_monitor_receive_device (udev_monitor)) == NULL)
+ sleep (1);
   if (matching_device (udev_device, devpath)) {
    type = udev_device_get_property_value (udev_device, "ID_FS_TYPE");
    if (type) {
---------- patch end ----------

But why need to handle udev_monitor_receive_device() errors?
I think that wait-for-root.c could be written as short as below code.

---------- simplified wait-for-root.c start ----------
#include <libudev.h>

#include <sys/types.h>
#include <sys/stat.h>

#include <stdio.h>
#include <limits.h>
#include <signal.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

static void alarm_handler(int signum)
{
 exit(1);
}

int main(int argc, char *argv[])
{
 const char *devpath;
 char path[PATH_MAX];
 struct udev *udev;

 if (argc != 3) {
  fprintf(stderr, "Usage: %s DEVICE TIMEOUT\n", argv[0]);
  exit(2);
 }

 devpath = argv[1];
 if (!strncmp(devpath, "UUID=", 5))
  snprintf(path, sizeof(path), "/dev/disk/by-uuid/%s", devpath + 5);
 else if (!strncmp(devpath, "LABEL=", 6))
  snprintf(path, sizeof(path), "/dev/disk/by-label/%s", devpath + 6);
 else
  snprintf(path, sizeof(path), "%s", devpath);

 signal(SIGALRM, alarm_handler);
 alarm(atoi(argv[2]));

 udev = udev_new();
 while (1) {
  struct stat devstat;
  const char *type;
  struct udev_device *udev_device;
  if (stat(path, &devstat) || !S_ISBLK(devstat.st_mode)) {
   sleep(1);
   continue;
  }
  udev_device = udev_device_new_from_devnum(udev, 'b', devstat.st_rdev);
  if (!udev_device) {
   sleep(1);
   continue;
  }
  type = udev_device_get_property_value(udev_device, "ID_FS_TYPE");
  if (type)
   printf("%s\n", type);
  udev_device_unref(udev_device);
  if (type)
   break;
 }
 udev_unref(udev);
 exit(0);
}
---------- simplified wait-for-root.c end ----------

[Tetsuo Handa (9-launchpad-i-love-sakura-ne-jp)]

Similar report in systemd package. https://bugzilla.redhat.com/show_bug.cgi?id=655857

[Martin Pitt (pitti)]

Thanks for investigating this! I don't really like the "poll every 1 s" approach, as in the worst case this would unnecessarily delay the boot by a whole second. But I agree that we can wait for 1 s if udev_monitor_receive_device() fails. There can't be an infinite loop as we always have the alarm_handler.

[Martin Pitt (pitti)]

Fix uploaded. Thanks!

[Launchpad Janitor (janitor)]

This bug was fixed in the package initramfs-tools - 0.103ubuntu1

---------------
initramfs-tools (0.103ubuntu1) saucy; urgency=low

  * src/wait-for-root.c: udev_monitor_receive_device() might still return NULL
    even with a blocking socket if recvmsg() fails with ENOBUFS. Retry every
    second in that case. Thanks to Tetsuo Handa for debugging this and the
    patch! (LP: #1215911)
 -- Martin Pitt <email address hidden> Mon, 26 Aug 2013 14:23:10 +0200

[Tetsuo Handa (9-launchpad-i-love-sakura-ne-jp)]

Thank you.

By the way, is there any possibility that "the message of a block device which
the wait-for-root is waiting for" cannot be received after ENOBUFS?

I think that any messages of block devices which are generated while the socket
buffer is full cannot be received using recvmsg(). I worry that "the message of
a block device which the wait-for-root is waiting for" is by chance generated
as one of such messages.

If such case occurs, waiting at udev_monitor_receive_device() will not return
"the message of a block device which the wait-for-root is waiting for", and
wait-for-root will uselessly waits until interrupted by SIGALRM. (The boot
would succeed anyway because the device file would have been created by the
time interrupted by SIGALRM. But really useless delay...)

[Martin Pitt (pitti)]

Tetsuo Handa [2013-08-26 12:59 -0000]:
> By the way, is there any possibility that "the message of a block device which
> the wait-for-root is waiting for" cannot be received after ENOBUFS?

I have never actually seen ENOBUFS, or uevents being missed due to it,
so I think the chance of that is quite small. But I can't assert that
all messages will be received after an ENOBUFS. But as you said,
waiting longer in that case is a safer fallback than not waiting at
all. My hope is that that ENOBUFS situation clears itself up
automatically after some time, otherwise your whole system would be
screwed (as you could never receive any uevent).

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

[Marc Deslauriers (mdeslaur)]

ACK on the debdiffs, they look good.

I've uploaded them for processing by the SRU team, with a slight change to the quantal versioning.

Thanks!

[Brian Murray (brian-murray)]

Hello Tetsuo, or anyone else affected,

Accepted initramfs-tools into raring-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/initramfs-tools/0.103ubuntu0.8 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Tetsuo, or anyone else affected,

Accepted initramfs-tools into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/initramfs-tools/0.103ubuntu0.2.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Tetsuo, or anyone else affected,

Accepted initramfs-tools into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/initramfs-tools/0.99ubuntu13.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Tetsuo Handa (9-launchpad-i-love-sakura-ne-jp)]

Martin Pitt (pitti) wrote on 2013-08-26:
> I have never actually seen ENOBUFS, or uevents being missed due to it,
> so I think the chance of that is quite small. But I can't assert that
> all messages will be received after an ENOBUFS. But as you said,
> waiting longer in that case is a safer fallback than not waiting at
> all. My hope is that that ENOBUFS situation clears itself up
> automatically after some time, otherwise your whole system would be
> screwed (as you could never receive any uevent).

I think that you can observe ENOBUFS and target uevents being missed due to it
if you try below change. I confirmed using below change that wait-for-root
waits until SIGALRM if wait-for-root failed to receive target uevents. Current
code assumes that socket buffer size is large enough to queue target uevents.

Anyway, although the possibility that wait-for-root waits longer than it should
is remaining, the possibility that wait-for-root waits shorter than it should
was fixed.

Thank you.

----------
--- a/src/wait-for-root.c
+++ b/src/wait-for-root.c
@@ -12,6 +12,9 @@
 #include <unistd.h>
 #include <fcntl.h>

+#include <sys/socket.h>
+#include <libudev.h>
+#include <errno.h>

 static int device_queued (struct udev *udev, const char *path);
 static int matching_device (struct udev_device *device, const char *path);
@@ -60,6 +63,11 @@ main (int argc,
   */
  udev = udev_new ();
  udev_monitor = udev_monitor_new_from_netlink (udev, "udev");
+ {
+ // Reduce socket buffer size.
+ int buff_size = 4096;
+ setsockopt(udev_monitor_get_fd(udev_monitor), SOL_SOCKET, SO_RCVBUF, &buff_size, sizeof(buff_size));
+ }

  udev_monitor_filter_add_match_subsystem_devtype (udev_monitor, "block", NULL);
  udev_monitor_enable_receiving (udev_monitor);
@@ -96,11 +104,15 @@ main (int argc,
  /* When the device doesn't exist yet, or is still being processed
   * by udev, use the monitor socket to wait it to be done.
   */
+ sleep(3); // Inject delay to make socket buffer overflow.
  while (1) {
                 /* even though we use a blocking socket this might still fail
                  * due to ENOBUFS or similar. */
- while ((udev_device = udev_monitor_receive_device (udev_monitor)) == NULL)
- sleep (1);
+ while (errno = 0, (udev_device = udev_monitor_receive_device (udev_monitor)) == NULL) {
+ const int err = errno;
+ fprintf(stderr, "***** %s (%d)\n", strerror(err), err);
+ //sleep (1);
+ }
   if (matching_device (udev_device, devpath)) {
    type = udev_device_get_property_value (udev_device, "ID_FS_TYPE");
    if (type) {
----------

[Brian Murray (brian-murray)]

@Dave - What kind of testing did you do to verify this SRU?

[Dave Chiluk (chiluk)]

An end-user that was experiencing the problem was able to verify that this fix solved his issue on precise. Unfortunately, as I am unable to reproduce all I could do was verify that initramfs-tools did not cause a noticeable regression on q,r.

[Launchpad Janitor (janitor)]

This bug was fixed in the package initramfs-tools - 0.99ubuntu13.3

---------------
initramfs-tools (0.99ubuntu13.3) precise; urgency=low

  * src/wait-for-root.c: udev_monitor_receive_device() might still
    return NULL even with a blocking socket if recvmsg() fails with
    ENOBUFS. Retry every second in that case. Thanks to Tetsuo Handa for
    debugging this and the patch! (LP: #1215911)
 -- Chris J Arges <email address hidden> Thu, 05 Sep 2013 16:15:26 -0500

[Steve Langasek (vorlon)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package initramfs-tools - 0.103ubuntu0.2.1

---------------
initramfs-tools (0.103ubuntu0.2.1) quantal; urgency=low

  * src/wait-for-root.c: udev_monitor_receive_device() might still
    return NULL even with a blocking socket if recvmsg() fails with
    ENOBUFS. Retry every second in that case. Thanks to Tetsuo Handa for
    debugging this and the patch! (LP: #1215911)
 -- Chris J Arges <email address hidden> Thu, 05 Sep 2013 16:17:52 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package initramfs-tools - 0.103ubuntu0.8

---------------
initramfs-tools (0.103ubuntu0.8) raring; urgency=low

  * src/wait-for-root.c: udev_monitor_receive_device() might still
    return NULL even with a blocking socket if recvmsg() fails with
    ENOBUFS. Retry every second in that case. Thanks to Tetsuo Handa for
    debugging this and the patch! (LP: #1215911)
 -- Chris J Arges <email address hidden> Thu, 05 Sep 2013 16:20:14 -0500

[Martin Pitt (pitti)]

Tetsuo,

Kay Sievers (udev upstream) just pointed out what to fix to avoid the ENOBUFS condition, I uploaded the fix to trusty:
https://launchpad.net/ubuntu/+source/initramfs-tools/0.103ubuntu3