curtin-dev team mailing list archive

[Olivier Gayot <mp+454608@xxxxxxxxxxxxxxxxxx>]

Olivier Gayot has proposed merging ~ogayot/curtin:efibootmgr into curtin:master.

Commit message:
On systems affected by the statfs kernel bug for efivarfs, export the LIBEFIVAR_OPS=efivarfs variable to work around the issue.

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/454608

A regression was introduced in the Linux 6.5 kernel affecting systems where the QueryVariableInfo UEFI function is not available. This includes various systems with Apple hardware.

On affected systems, calls to statfs("/sys/firmware/efi/efivarfs") fail with EINVAL. As a consequence, libefivar (and therefore efibootmgr!) assume that EFI variables are not supported.

Command: ['efibootmgr', '-v']
Exit code: 2
Reason: -
Stdout: ''
Stderr: EFI variables are not supported on this system.

Because the root cause is being addressed at the kernel level (see bug 2034705), the proper fix cannot easily be deployed without distributing new isos.

In the meantime, it would be ideal for affected users to benefit from a workaround if one can be delivered via the subiquity refresh mechanism.

This is the purpose of this change.

Some testing on affected systems would be needed before merging.
-- 
Your team curtin developers is requested to review the proposed merge of ~ogayot/curtin:efibootmgr into curtin:master.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 10a12bd..4be2cb4 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -1919,6 +1919,8 @@ def curthooks(args):
     stack_prefix = state.get('report_stack_prefix', '')
     curthooks_mode = cfg.get('curthooks', {}).get('mode', 'auto')
 
+    util.EFIVarFSBug.apply_workaround_if_affected()
+
     # UC is special, handle it first.
     if distro.is_ubuntu_core(target):
         LOG.info('Detected Ubuntu-Core image, running hooks')
diff --git a/curtin/commands/install_grub.py b/curtin/commands/install_grub.py
index 285e6a5..18bc33d 100644
--- a/curtin/commands/install_grub.py
+++ b/curtin/commands/install_grub.py
@@ -467,6 +467,9 @@ def install_grub_main(args):
     cfg = config.load_command_config(args, state)
     stack_prefix = state.get('report_stack_prefix', '')
     uefi = util.is_uefi_bootable()
+
+    util.EFIVarFSBug.apply_workaround_if_affected()
+
     grubcfg = config.fromdict(config.GrubConfig, cfg.get('grub'))
     with events.ReportEventStack(
             name=stack_prefix, reporting_enabled=True, level="INFO",
diff --git a/curtin/util.py b/curtin/util.py
index 3e54795..2a0899c 100644
--- a/curtin/util.py
+++ b/curtin/util.py
@@ -904,6 +904,47 @@ def is_uefi_bootable():
     return os.path.exists('/sys/firmware/efi') is True
 
 
+class EFIVarFSBug:
+    """A regression was introduced in the Linux 6.5 kernel affecting systems
+    where the QueryVariableInfo UEFI function is not available. This includes
+    various systems with Apple hardware.
+    On affected systems, calls to statfs("/sys/firmware/efi/efivarfs") fail
+    with EINVAL. As a consequence, libefivar (and therefore efibootmgr!) assume
+    that EFI variables are not supported.
+
+    Command: ['efibootmgr', '-v']
+    Exit code: 2
+    Reason: -
+    Stdout: ''
+    Stderr: EFI variables are not supported on this system.
+
+    Fortunately, libefivar supports the LIBEFIVAR_OPS variable, which can be
+    used to "force" the library to use "/sys/firmware/efi/efivarfs" without
+    relying on statfs(2).
+
+    See LP: #2040190"""
+    @staticmethod
+    def is_system_affected(check_efi_bootable=True) -> bool:
+        """Try to determine is the bug affects the current system."""
+        try:
+            os.statvfs("/sys/firmware/efi/efivarfs")
+        except OSError as ose:
+            if ose.errno == errno.EINVAL:
+                return True
+        except Exception:
+            pass
+        return False
+
+    @staticmethod
+    def apply_workaround() -> None:
+        os.environ["LIBEFIVAR_OPS"] = "efivarfs"
+
+    @staticmethod
+    def apply_workaround_if_affected() -> None:
+        if EFIVarFSBug.is_system_affected():
+            EFIVarFSBug.apply_workaround()
+
+
 @attr.s(auto_attribs=True)
 class EFIBootEntry:
     name: str
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index e2c4caf..6bbe151 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -1,6 +1,7 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
 from unittest import mock
+import errno
 import os
 import stat
 from textwrap import dedent
@@ -1195,4 +1196,48 @@ class TestNotExclusiveRetry(CiTestCase):
             util.not_exclusive_retry(f, 1, 2, 3)
         sleep.assert_called_once()
 
+
+class TestEFIVarFSBug(CiTestCase):
+    def test_is_system_affected__unaffected(self):
+        # os.statvfs succeeds
+        with mock.patch("os.statvfs", return_value=None):
+            self.assertFalse(util.EFIVarFSBug.is_system_affected())
+
+        # os.statvfs fails with PermissionError (e.g., EPERM)
+        with mock.patch("os.statvfs", side_effect=PermissionError):
+            self.assertFalse(util.EFIVarFSBug.is_system_affected())
+
+        # os.statvfs fails with FileNotFoundError (e.g., ENOENT)
+        with mock.patch("os.statvfs", side_effect=FileNotFoundError):
+            self.assertFalse(util.EFIVarFSBug.is_system_affected())
+
+        # os.statvfs fails with OSError (but not EINVAL)
+        ose = OSError(errno.EBADFD, "Bad file descriptor")
+        with mock.patch("os.statvfs", side_effect=ose):
+            self.assertFalse(util.EFIVarFSBug.is_system_affected())
+
+    def test_is_system_affected__affected(self):
+        ose = OSError(errno.EINVAL, "Invalid argument")
+        with mock.patch("os.statvfs", side_effect=ose):
+            self.assertTrue(util.EFIVarFSBug.is_system_affected())
+
+    def test_apply_workaround(self):
+        with mock.patch.dict(os.environ, clear=True):
+            util.EFIVarFSBug.apply_workaround()
+            self.assertEqual(os.environ["LIBEFIVAR_OPS"], "efivarfs")
+
+    def test_apply_workaround_if_affected(self):
+        with mock.patch.object(util.EFIVarFSBug,
+                               "apply_workaround") as apply_wa:
+            with mock.patch.object(util.EFIVarFSBug, "is_system_affected",
+                            return_value=True):
+                util.EFIVarFSBug.apply_workaround_if_affected()
+            apply_wa.assert_called_once()
+
+            apply_wa.reset_mock()
+            with mock.patch.object(util.EFIVarFSBug, "is_system_affected",
+                            return_value=False):
+                util.EFIVarFSBug.apply_workaround_if_affected()
+            apply_wa.assert_not_called()
+
 # vi: ts=4 expandtab syntax=python