/etc/grub.d/10_linux_zfs gives up if any checked /etc/fstab entry is invalid

Bug #1849347 reported by Deltik
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
grub2 (Ubuntu)
Fix Released
High
Jean-Baptiste Lallement
Eoan
Won't Fix
High
Jean-Baptiste Lallement
Focal
Fix Released
High
Jean-Baptiste Lallement
grubzfs-testsuite (Ubuntu)
Fix Released
High
Jean-Baptiste Lallement
Eoan
Won't Fix
High
Jean-Baptiste Lallement
Focal
Fix Released
High
Jean-Baptiste Lallement

Bug Description

[Impact]
Ubuntu's ZFS boot support (`/etc/grub.d/10_linux_zfs`) will fail to generate any GRUB entries for ZFS roots if any of the checked `/etc/fstab` entries fails to mount. This includes all past ZFS snapshots.

[Test Case]
1. On a system that has a ZFS root (e.g. one installed with zsys), run `sudo update-grub`.

2. Verify that there are GRUB boot entries for ZFS:

    sed -n '/^### BEGIN \/etc\/grub\.d\/10_linux_zfs ###$/,/^### END \/etc\/grub\.d\/10_linux_zfs ###$/p;/^### END \/etc\/grub\.d\/10_linux_zfs ###$/q' /boot/grub/grub.cfg

3. To reproduce the problem, create or edit a `/boot` entry that will fail mounting. (Back up the existing line if you have one and are working on a real system.) Example:

    echo 'UUID=deaddead-dead-4ead-dead-deaddeaddead /boot ext2 discard,nofail 0 2' | sudo tee -a /etc/fstab

4. (Optional) Take a ZFS snapshot of the dataset that provides `/etc`:

    sudo zfs snap "$(df /etc -t zfs | sed '/^Filesystem/d' | awk '{print $1}')"@repro

5. If you performed step 4, restore the `/etc/fstab` back to its original working condition:

    sudo sed -i '/^UUID=deaddead-dead-4ead-dead-deaddeaddead/d' /etc/fstab

6. Run `sudo update-grub` again.

7. See that the GRUB boot entries for ZFS roots have been wiped out:

    sed -n '/^### BEGIN \/etc\/grub\.d\/10_linux_zfs ###$/,/^### END \/etc\/grub\.d\/10_linux_zfs ###$/p;/^### END \/etc\/grub\.d\/10_linux_zfs ###$/q' /boot/grub/grub.cfg

[Regression Potential]
The patch adds a sanity check for a mount that could fail. The sanity check prevents the entire `/etc/grub.d/10_linux_zfs` from being wiped out from that failed mount.

The new behavior causes the `get_system_directory()` function in `/etc/grub.d/10_linux_zfs` to act as if the bad mount didn't exist. The function would then continue to the next case, which is currently documented as "Handle zfs case, which can be a snapshots."

Tags: patch eoan
Revision history for this message
Deltik (deltik) wrote :
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "/etc/grub.d/10_linux_zfs: Ignore failed /etc/fstab mount in the get_system_directory() function" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Changed in grub2 (Ubuntu):
status: New → Triaged
importance: Undecided → High
assignee: nobody → Jean-Baptiste Lallement (jibel)
Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Thank you very much for the detailed bug report and the patch. We've applied your patch but removed the redirection to stderr to help understand the error in case of failure.

Changed in grub2 (Ubuntu Focal):
status: Triaged → In Progress
Changed in grub2 (Ubuntu Eoan):
status: New → Triaged
importance: Undecided → High
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in grubzfs-testsuite (Ubuntu Eoan):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in grubzfs-testsuite (Ubuntu Focal):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in grubzfs-testsuite (Ubuntu Eoan):
importance: Undecided → High
Changed in grubzfs-testsuite (Ubuntu Focal):
importance: Undecided → High
Changed in grubzfs-testsuite (Ubuntu Eoan):
status: New → Triaged
Changed in grubzfs-testsuite (Ubuntu Focal):
status: New → Triaged
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grubzfs-testsuite - 0.4.6

---------------
grubzfs-testsuite (0.4.6) focal; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  * Test cases for:
    - Handle the case where grub-probe returns several devices for a single
      pool (LP: #1848856).
    - Do not crash on invalid fstab and report the invalid entry.
      (LP: #1849347)
    - When a pool fails to import, catch and display the error message and
      continue with other pools. Import all the pools in readonly mode so we
      can import other pools with unsupported features (LP: #1848399)

 -- Jean-Baptiste Lallement <email address hidden> Mon, 18 Nov 2019 11:38:20 +0100

Changed in grubzfs-testsuite (Ubuntu Focal):
status: Triaged → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grub2 - 2.04-1ubuntu14

---------------
grub2 (2.04-1ubuntu14) focal; urgency=medium

  * debian/patches/ubuntu-zfs-enhance-support.patch:
    - Handle the case where grub-probe returns several devices for a single
      pool (LP: #1848856). Thanks jpb for the report and the proposed patch.
    - Add savedefault to non-recovery entries (LP: #1850202). Thanks Deltik
      for the patch.
    - Do not crash on invalid fstab and report the invalid entry.
      (LP: #1849347) Thanks Deltik for the patch.
    - When a pool fails to import, catch and display the error message and
      continue with other pools. Import all the pools in readonly mode so we
      can import other pools with unsupported features (LP: #1848399) Thanks
      satmandu for the investigation and the proposed patch

 -- Jean-Baptiste Lallement <email address hidden> Mon, 18 Nov 2019 11:22:43 +0100

Changed in grub2 (Ubuntu Focal):
status: In Progress → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote :

The Eoan Ermine has reached end of life, so this bug will not be fixed for that release

Changed in grub2 (Ubuntu Eoan):
status: Triaged → Won't Fix
Changed in grubzfs-testsuite (Ubuntu Eoan):
status: Triaged → Won't Fix
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.