users shoot themselves in the foot by removing /boot/efi from /etc/fstab; u-r-u should warn and refuse to let them upgrade

Bug #1695666 reported by Steve Langasek
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ubuntu-release-upgrader (Ubuntu)
Fix Released
High
Mathieu Trudel-Lapierre
Artful
Triaged
High
Mathieu Trudel-Lapierre

Bug Description

We currently have 6 open bug reports at <https://bugs.launchpad.net/ubuntu/+source/shim-signed>, and several closed ones, from users who have by one means or another gotten /boot/efi unmounted on their systems and then tried to apply updates.

On a UEFI system, /boot/efi must be a separate mount point (it's the EFI System Partition); and it must be mounted at the time of upgrading the shim-signed/grub-efi-amd64 packages, because it's meaningless to "upgrade" these packages without installing the new bootloader to the ESP - if you don't want to use the Ubuntu UEFI bootloader, uninstall the package.

However, a surprising number of users (i.e., more than 0) are leaving /boot/efi unmounted on their systems, and as a result get upgrade failures. This is a bad thing to have happen in the middle of a dist-upgrade between releases.

I think u-r-u should detect the case where either of the shim-signed and grub-efi-amd64 packages are installed, and /boot/efi is not in a sane state (is mountpoint + is mounted rw), and refuse to let the user start the upgrade until this is resolved.

Steve Langasek (vorlon)
Changed in ubuntu-release-upgrader (Ubuntu):
status: New → Triaged
importance: Undecided → High
assignee: nobody → Canonical Foundations Team (canonical-foundations)
milestone: none → ubuntu-17.10
tags: added: rls-aa-incoming
Revision history for this message
Adam Conrad (adconrad) wrote :

From the POV of "u-r-u shouldn't be a dumping ground for quirking upgrade issues we can fix in packages", shouldn't this check be in preinsts for the packages that twiddle /boot/efi, not in the upgrader?

Also, can we do better here (like we do with non-mounted bootloader partitions, like PReP) and somehow automatically detect the correct partition and mount it before attempting to use it?

Revision history for this message
Steve Langasek (vorlon) wrote : Re: [Bug 1695666] Re: users shoot themselves in the foot by removing /boot/efi from /etc/fstab; u-r-u should warn and refuse to let them upgrade

On Wed, Jun 07, 2017 at 06:08:25PM -0000, Adam Conrad wrote:
> >From the POV of "u-r-u shouldn't be a dumping ground for quirking
> upgrade issues we can fix in packages", shouldn't this check be in
> preinsts for the packages that twiddle /boot/efi, not in the upgrader?

No, because the preinst is also run in the middle of the upgrade and causes
the upgrade overall to fail.

> Also, can we do better here (like we do with non-mounted bootloader
> partitions, like PReP) and somehow automatically detect the correct
> partition and mount it before attempting to use it?

If you detect multiple ESPs, how do you decide which one to mount?

(It's possible that yes, we could work this out based on things like current
efibootmgr settings, but I think it would be somewhat error prone and you
would still want to detect it all before starting the dist-upgrade.)

blkid -t PARTUUID=$(efibootmgr -v | awk '/^Boot..... ubuntu\t/ { print $NF }' | cut -f3 -d,)

Steve Langasek (vorlon)
tags: removed: rls-aa-incoming
Steve Langasek (vorlon)
Changed in ubuntu-release-upgrader (Ubuntu Artful):
assignee: Canonical Foundations Team (canonical-foundations) → Mathieu Trudel-Lapierre (cyphermox)
tags: added: id-597a8306d5b3e697ea8b96a5
Revision history for this message
Julian Andres Klode (juliank) wrote :

Patch, needs review (especially of the new strings), and some testing - I only checked the function externally.

Changed in ubuntu-release-upgrader (Ubuntu Artful):
status: Triaged → Fix Committed
tags: added: patch
Revision history for this message
Steve Langasek (vorlon) wrote :

For completeness we should also check that the filesystem mounted at /boot/efi is of type vfat.

+ _("The UEFI Boot partition of your system "
+ "is not mounted on /boot/efi. "
+ "Please ensure that it is properly configured "
+ "and retry."))

Suggested wording:

Your EFI System Partition (ESP) is not mounted at /boot/efi. Please ensure that it is properly configured and try again.

+ else:
+ self._view.error(_("UEFI Boot partition not usable"),
+ _("The UEFI Boot partition of your system "
+ "(/boot/efi) is not writable. "
+ "Please ensure that it is properly configured "
+ "and retry."))

The EFI System Partition (ESP) mounted at /boot/efi is not writable. Please mount this partition read-write and try again.

Changed in ubuntu-release-upgrader (Ubuntu Artful):
status: Fix Committed → Won't Fix
Changed in ubuntu-release-upgrader (Ubuntu):
status: Triaged → Fix Committed
Changed in ubuntu-release-upgrader (Ubuntu Artful):
status: Won't Fix → Triaged
Revision history for this message
Julian Andres Klode (juliank) wrote :

Checking for vfat seems fragile. You could, like systemd does upstream, also mount it as an autofs with a short timeout.

But yeah, the texts are better than mine. Though I do fear that people do not know what an EFI system partition is.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-release-upgrader - 1:18.04.6

---------------
ubuntu-release-upgrader (1:18.04.6) bionic; urgency=medium

  * DistUpgradeFetcherKDE.py:
    - ensure theming is correct across all Qt versions and Plasma versions
    - drop unused openUrl helper (replaced by QUrlOpener)
  * QUrlOpener.py:
    - make sure to pass --set-home to sudo when dropping back to user, prevents
      browsers from getting confused which HOME to use
  * DistUpgradeViewKDE.py:
    - force KDE theming for Qt5
    - fix icon & pixmap lookup by running them throw a compatibility helper
      using either Qt3/4 static lookup or Qt5 theme-based lookup (theme is
      determined by the QPA ideally)
    - let name mangling use os-release' PRETTY_NAME if present
    - do not resize windows, adjustsize() them (same as resize albeit readable)
    - introduce an override for QWidget.adjustSize() which prevents adjustment
      iff the window is maximized. as noted in the documentation adjusting a
      maximized yields unexpected results due to a control mismatch between
      X11 window managers and Qt (in short: Qt cannot unmaximize, so adjustsize
      does nothing useful in this case and in fact causes only part of the
      window to be marked dirty by Qt causing sever rendering artifacts)

 -- Harald Sitter <email address hidden> Fri, 19 Jan 2018 13:34:32 +0100

Changed in ubuntu-release-upgrader (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Julian Andres Klode (juliank) wrote :

The changelog entry is the wrong one but it's fixed. String improvements are queued in bzr but some tests fail and need fixing :(

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.