umountfs must check whether a mountpoint contains a loopmounted root file

[Agostino Russo]

Binary package hint: initscripts

In a loopinstallation unmounting the /host partition (or a bindmounted partition that implies unmounting the /host partition) will result in an hang when rebooting.

[Agostino Russo (ago)]

Patch provided

66,67d65
< loop_file=$(awk '$2=="/" && $4~"loop" {print $1}' /etc/fstab)
< host_device=$(awk '"'${loop_file}'"~"^"$2 && $2!="/" {print $1}' /proc/mounts)
70,71c68
< [ "$DEV" = "'$host_device'" ] && continue
< case "$MTPT" in
---
> case "$MTPT" in

[Agostino Russo (ago)]

As suggested by Colin Watson, a better fix would be to skip every device that appears in /proc/mounts before / does. I will provide a second patch to that effect, the above only skips loop installations.

[Agostino Russo (ago)]

Perhaps the following will do

root_line=$(awk '$1!~"^rootfs" && $2=="/" {print NR}' /proc/mounts)
skip_devices=$(head -n $root_line /proc/mounts | cut -d ' ' -f 1)
....

echo $skip_devices | grep -qs $DEV"$" && continue

[Agostino Russo (ago)]

Of course umountroot also needs to be patched so that any remaining host filesystems are unmounted/remounted ro. That is quite similar to run umountfs (without the above patch) again after umountroot.

[Agostino Russo (ago)]

preliminary patch for umountroot: instead of remounting ro only /, extend the same treatment to any mountpoint still mentioned in /proc/mounts

remount_ro(){
    local MTPT=$1
    [ "$VERBOSE" = no ] || log_action_begin_msg "Mounting $MPT read-only"
    MOUNT_FORCE_OPT=
    [ "$(uname -s)" = "GNU/kFreeBSD" ] && MOUNT_FORCE_OPT=-f
    # This:
    # mount -n -o remount,ro /
    # will act on a bind mount of / if there is one.
    # See #339023 and the comment in checkroot.sh
    mount $MOUNT_FORCE_OPT -n -o remount,ro -t dummytype dummydev $MTPT 2>/dev/null \
        || mount $MOUNT_FORCE_OPT -n -o remount,ro dummydev $MTPT 2>/dev/null \
        || mount $MOUNT_FORCE_OPT -n -o remount,ro $MTPT
    ES=$?
    [ "$VERBOSE" = no ] || log_action_end_msg $ES
}

do_stop () {
    # These directories must exist on the root filesystem as they are
    # targets for system mountpoints. We've just unmounted all other
    # filesystems, so either they are mounted now (in which case the
    # mount point exists) or we can make the mountpoint.
    for dir in /proc /sys /var/run /var/lock; do
        mkdir -p $dir || true
    done
    exec 9<&0 </proc/mounts
    REG_MTPTS=""
    TMPFS_MTPTS=""
    while read DEV MTPT FSTYPE REST
    do
        case "$MTPT" in
          /proc|/dev|/.dev|/dev/pts|/dev/shm|/proc/*|/sys|/var/run|/var/lock)
            continue
            ;;
        esac
        case "$FSTYPE" in
          proc|procfs|linprocfs|devfs|sysfs|usbfs|usbdevfs|devpts)
            continue
            ;;
        *)
            remount_ro $MTPT
            ;;
        esac
    done
    exec 0<&9 9<&-
}

[Agostino Russo (ago)]

As suggested by Colin Watson, a better way to skip mountpoints in umountfs is using the following code:

do_stop () {
 exec 9<&0 < <(sed -n '/^\/[^ ]* \/ /,$p' /proc/mounts)

 REG_MTPTS=""
 TMPFS_MTPTS=""
 while read DEV MTPT FSTYPE REST;
 do
        echo "Processing $DEV $MTPT"
        done

    exec 0<&9 9<&-
}

[Launchpad Janitor (janitor)]

This bug was fixed in the package sysvinit - 2.86.ds1-14.1ubuntu33

---------------
sysvinit (2.86.ds1-14.1ubuntu33) hardy; urgency=low

  * Don't unmount filesystems that precede root (LP: #151579). Thanks
    Agostino Russo and Colin Watson.

 -- Evan Dandrea <email address hidden> Thu, 17 Jan 2008 10:50:41 -0500

[Agostino Russo (ago)]

Still not working. The issue is due to the use of umount -f -d. The previous patch basically skipped the mountpoints (as opposed to skipping devices). But umount -f -d also affects the underlying devices (e.g. in bindmounts). The proposed solutions skips the mountpoints listed at the top of /proc/mounts (as before) and for the rest it changes the flags whenever the affected device is in the top of /proc/mounts.

--- /etc/init.d/umountfs 2007-10-04 12:17:31.000000000 +0100
+++ umountfs 2008-01-25 11:14:42.770493000 +0000
@@ -60,18 +60,20 @@

 do_stop () {
  exec 9<&0 </proc/mounts
-
+ PROTECTED_MOUNTS="$(sed -n '0,/^\/[^ ]* \/ /p' /proc/mounts)"
+ PROTECTED_MTPTS=""
  REG_MTPTS=""
  TMPFS_MTPTS=""
  while read DEV MTPT FSTYPE REST
  do
+ echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV $MTPT " && continue
   case "$MTPT" in
     /|/proc|/dev|/.dev|/dev/pts|/dev/shm|/proc/*|/sys|/var/run|/var/lock)
    continue
    ;;
   esac
- case "$FSTYPE" in
- proc|procfs|linprocfs|devfs|sysfs|usbfs|usbdevfs|devpts)
+ case "$FSTYPE" in
+ proc|procfs|linprocfs|devfs|sysfs|usbfs|usbdevfs|devpts|securityfs)
    continue
    ;;
     tmpfs)
@@ -81,12 +83,15 @@
    ;;
     *)
    REG_MTPTS="$REG_MTPTS $MTPT"
+ if echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV "; then
+ PROTECTED_MTPTS="$PROTECTED_MTPTS $MTPT "
+ fi
    ;;
   esac
  done

  exec 0<&9 9<&-
-
+
  #
  # Make sure tmpfs file systems are umounted before turning off
  # swap, to avoid running out of memory if the tmpfs filesystems
@@ -140,19 +145,31 @@
   REG_MTPTS="$(pioodl $REG_MTPTS)"
   if [ "$VERBOSE" = no ]
   then
- log_action_begin_msg "Unmounting local filesystems"
- umount -f -r -d $REG_MTPTS
- log_action_end_msg $?
+ for MTPT in $REG_MTPTS; do
+ log_action_begin_msg "Unmounting local filesystem $MTPT"
+ if echo "PROTECTED_MTPTS" | grep -qs " $MTPT "; then
+ umount -r $MTPT
+ else
+ umount -f -r -d $MTPT
+ fi
+ log_action_end_msg $?
+ done
   else
- log_action_msg "Will now unmount local filesystems"
- umount -f -v -r -d $REG_MTPTS
- ES=$?
- if [ "$ES" = 0 ]
- then
- log_success_msg "Done unmounting local filesystems."
- else
- log_failure_msg "Unmounting local filesystems failed with error code ${ES}."
- fi
+ for MTPT in $REG_MTPTS; do
+ log_action_begin_msg "Will now unmount local filesystem $MTPT"
+ if echo "PROTECTED_MTPTS" | grep -qs " $MTPT "; then
+ umount -r -v $MTPT
+ else
+ umount -f -r -d -v $MTPT
+ fi
+ ES=$?
+ if [ "$ES" = 0 ]
+ then
+ log_success_msg "Done unmounting local filesystem $MTPT."
+ else
+ log_failure_msg "Unmounting local filesystem $MTPT failed with error code ${ES}."
+ fi
+ done
   fi
  fi
 }

[Agostino Russo (ago)]

In fact only the -f flag is problematic. See new patch which is a bit less invasive

--- /etc/init.d/umountfs 2007-10-04 12:25:08.000000000 +0100
+++ umountfs 2008-01-26 07:20:48.000000000 +0000
@@ -60,18 +60,20 @@

 do_stop () {
  exec 9<&0 </proc/mounts
-
+ PROTECTED_MOUNTS="$(sed -n '0,/^\/[^ ]* \/ /p' /proc/mounts)"
+ WEAK_MTPTS="" #be gentle, don't use force
  REG_MTPTS=""
  TMPFS_MTPTS=""
  while read DEV MTPT FSTYPE REST
  do
+ echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV $MTPT " && continue
   case "$MTPT" in
     /|/proc|/dev|/.dev|/dev/pts|/dev/shm|/proc/*|/sys|/var/run|/var/lock)
    continue
    ;;
   esac
   case "$FSTYPE" in
- proc|procfs|linprocfs|devfs|sysfs|usbfs|usbdevfs|devpts)
+ proc|procfs|linprocfs|devfs|sysfs|usbfs|usbdevfs|devpts|securityfs)
    continue
    ;;
     tmpfs)
@@ -81,6 +83,9 @@
    ;;
     *)
    REG_MTPTS="$REG_MTPTS $MTPT"
+ if echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV "; then
+ WEAK_MTPTS="$WEAK_MTPTS $MTPT "
+ fi
    ;;
   esac
  done
@@ -138,22 +143,29 @@
  if [ "$REG_MTPTS" ]
  then
   REG_MTPTS="$(pioodl $REG_MTPTS)"
- if [ "$VERBOSE" = no ]
- then
- log_action_begin_msg "Unmounting local filesystems"
- umount -f -r -d $REG_MTPTS
- log_action_end_msg $?
- else
- log_action_msg "Will now unmount local filesystems"
- umount -f -v -r -d $REG_MTPTS
- ES=$?
- if [ "$ES" = 0 ]
+ for MTPT in $REG_MTPTS; do
+ if echo "$WEAK_MTPTS" | grep -qs " $MTPT "; then
+ FORCE=""
+ else
+ FORCE="-f"
+ fi
+ if [ "$VERBOSE" = no ]
    then
- log_success_msg "Done unmounting local filesystems."
+ log_action_begin_msg "Unmounting local filesystem $MTPT"
+ umount $FORCE -r -d $MTPT
+ log_action_end_msg $?
    else
- log_failure_msg "Unmounting local filesystems failed with error code ${ES}."
+ log_action_msg "Will now unmount local filesystem $MTPT"
+ umount $FORCE -v -r -d $MTPT
+ ES=$?
+ if [ "$ES" = 0 ]
+ then
+ log_success_msg "Done unmounting local filesystem $MTPT."
+ else
+ log_failure_msg "Unmounting local filesystem $MTPT failed with error code ${ES}."
+ fi
    fi
- fi
+ done
  fi
 }

[Agostino Russo (ago)]

To keep verbosity in line with original file see also
http://paste.ubuntu-nl.org/53974/

[Launchpad Janitor (janitor)]

This bug was fixed in the package sysvinit - 2.86.ds1-14.1ubuntu34

---------------
sysvinit (2.86.ds1-14.1ubuntu34) hardy; urgency=low

  * Don't use force for some mountpoints in umountfs (LP: #151579).
    Thanks Agostino Russo.

 -- Evan Dandrea <email address hidden> Mon, 28 Jan 2008 11:31:13 -0500

[Paul Albrecht (albrecht-rdi1)]

Did the fix make it into beta? I have the following line at the start of the do loop:

echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV $MTPT " && continue

This obviously doesn't work because the subsequent case statements that construct TMPFS_MTPTS and REG_MTPTS aren't executed when the loop is continued.

[Agostino Russo (ago)]

Paul,

Basically $PROTECTED_MOUNTS contains all mountpoints which were mounted before root was mounted. As such they might host root. Such mountpoints should not be unmounted in umountfs. They are handled instead in umountroot.

[Paul Albrecht (albrecht-rdi1)]

OK, Thanks, I get that, but I'm questioning the way do_stop is coded. It looks to me like the the loop in do_stop skips over *all* entries in /proc/mounts, but that doesn't make sense because it's iterating over entries in /proc/mounts. So what's the point of the loop?

[Agostino Russo (ago)]

It should only skip entries listed in $PROTECTED_MOUNTS, which is a subset of /proc/mounts, given by:

sed -n '0,/^\/[^ ]* \/ /p' /proc/mounts

[Paul Albrecht (albrecht-rdi1)]

Which entries are you trying to skip over with $PROTECTED_MOUNTS? If I do sed -n '0,/^\/[^ ]* \/ /p' /proc/mounts, I get this:

rootfs / rootfs rw 0 0
none /sys sysfs rw,nosuid,nodev,noexec 0 0
none /proc proc rw,nosuid,nodev,noexec 0 0
udev /dev tmpfs rw,relatime 0 0
fusectl /sys/fs/fuse/connections fusectl rw,relatime 0 0
/dev/disk/by-uuid/96130139-f845-4f68-bb06-92a34955905f / ext3 rw,relatime,errors=remount-ro,data=ordered 0 0

[Agostino Russo (ago)]

All the ones before (and including) /

That is required in situations where / is inside another device (e.g. / is inside /host/path/to/root.containing.loopfile where /dev/sdXY is mounted as /host), otherwise unmounting /host would be equivalent to unmounting /. Host devices will appear before / in /proc/mounts. /etc/init.d/umountroot has also been modified to remount r/o all such devices.

[Agostino Russo (ago)]

One thing worth considering is allowing the list of TMPFS_MTPTS filesystems to be compiled before skipping PROTECTED_MOUNTS. While there is a rationale for skipping REG_MTPTS, that does not extend to TMPFS_MTPTS.

[Paul Albrecht (albrecht-rdi1)]

I understand you can't umount the loop device when it's root, but wouldn't it be simpler to get the loop device from /proc/cmdline and skip it specifically in do_stop?

[Paul Albrecht (albrecht-rdi1)]

Actually, it would be simpler still to add the /host directory to the set of excluded mount points at the start of the do loop because it's hardcoded in initrd as the mount point for the original root filesystem in the loopback file system whenever you specify the loop install boot option. It can't really be used for any other purpose so I'm not sure it's worth checking /proc/cmdline for the loop install boot option. What do you think?

[Agostino Russo (ago)]

Paul, absolutely, that would work, but it would also be less generic.

[Agostino Russo (ago)]

To be more precise I do not have any strong preference, provided loopinstallations can be unmounted safely. It is not up to me to decide what version to use, you might want to ask Colin Watson and Evan Dandrea. Note that, as explained above, WEAK_MTPTS would still need to be there (they could also be discovered hardcoding "/host").

[Agostino Russo (ago)]

By the way, if you see the first 2 comments, you will notice that my original proposal was not too dissimilar from yours. It was Colin suggestion to generalize the algorithm instead of making it wubi-specific.

[Agostino Russo (ago)]

To avoid skipping early tmpfs mounts:

--- umountfs 2008-04-01 21:05:35.000000000 +0100
+++ umountfs.new 2008-04-01 21:13:16.000000000 +0100
@@ -66,7 +66,9 @@
  TMPFS_MTPTS=""
  while read DEV MTPT FSTYPE REST
  do
- echo "$PROTECTED_MOUNTS" | grep -qs "^$DEV $MTPT " && continue
+ if "$PROTECTED_MOUNTS" | grep -qs "^$DEV $MTPT "; then
+ [ "$FSTYPE" != tmpfs ] && continue
+ fi
   case "$MTPT" in
    /|/proc|/dev|/.dev|/dev/pts|/dev/shm|/proc/*|/sys|/var/run|/var/lock)
    continue