From 75c9939f84a438e288b21183b70f62f3e87568d5 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Fri, 29 Nov 2024 11:12:38 -0800 Subject: [PATCH] xorriso: respect efiparttable and gpt_hybrid_mbr This should make the xorriso-based ISO build path respect the 'efiparttable' and 'gpt_hybrid_mbr' settings when building a UEFI-compatible image, making it write a GPT disk label by default instead of an MBR (msdos) one. If it's building an image that is not UEFI-compatible it will always write an MBR label, regardless of this setting. If 'gpt_hybrid_mbr' is set, xorriso will write an Ubuntu-style MBR/GPT hybrid partition table, where the MBR partition table includes a partition with type 00 and the bootable flag, as well as the partition with type ee required by the UEFI spec. This mildly violates the UEFI spec but may make the image bootable on native BIOS or CSM firmwares which refuse to boot from a disk with no partition marked 'bootable' in the MBR. If 'gpt_hybrid_mbr' is not set, xorriso will write a strictly UEFI-spec compliant label, with just the 'protective MBR' required by the UEFI spec (no bootable partition) and the correct GPT partition table. Note this is somewhat different from what gpt_hybrid_mbr does for disk images. Also, we now pass -compliance no_emul_toc when building ISOs, as recommended by upstream in https://lists.gnu.org/archive/html/bug-xorriso/2024-11/msg00012.html This tool is generally always going to be building ISOs intended for write-once use, not multi-session use (and which are rarely, these days, written to physical discs at all anyway). Signed-off-by: Adam Williamson --- doc/source/image_description/elements.rst | 11 +++- kiwi/builder/install.py | 2 + kiwi/builder/live.py | 2 + kiwi/filesystem/isofs.py | 2 +- kiwi/firmware.py | 1 + kiwi/iso_tools/base.py | 6 +- kiwi/iso_tools/xorriso.py | 28 ++++++++- kiwi/schema/kiwi.rnc | 4 +- kiwi/schema/kiwi.rng | 4 +- test/unit/builder/live_test.py | 4 ++ test/unit/filesystem/isofs_test.py | 3 +- test/unit/iso_tools/xorriso_test.py | 71 +++++++++++++++++++---- 12 files changed, 115 insertions(+), 23 deletions(-) diff --git a/doc/source/image_description/elements.rst b/doc/source/image_description/elements.rst index d8dcb5a87ef..6f9fbe779a6 100644 --- a/doc/source/image_description/elements.rst +++ b/doc/source/image_description/elements.rst @@ -402,7 +402,7 @@ efifatimagesize="nonNegativeInteger": efiparttable="msdos|gpt": For images with an EFI firmware specifies the partition table type to use. If not set defaults to the GPT partition - table type + table type for disk images, MBR (msdos) for ISO images. dosparttable_extended_layout="true|false": For oem disk images, specifies to make use of logical partitions @@ -889,7 +889,14 @@ force_mbr="true|false": partitions gpt_hybrid_mbr="true|false": - For GPT disk types only: Create a hybrid GPT/MBR partition table + For disk types, create a hybrid GPT/MBR partition table with an + 'accurate' MBR table that will have no 'bootable' flagged partition + For ISO types, create a hybrid GPT/MBR partition table where the + MBR partition table contains a whole-disk 'protective' partition + and a second bootable-flagged partition (intended to make the image + bootable in both UEFI and BIOS modes on as much hardware as possible) + In both cases, only has any effect if the EFI partition table + type is GPT hybridpersistent="true|false": For the live ISO type, triggers the creation of a partition for diff --git a/kiwi/builder/install.py b/kiwi/builder/install.py index bebf16ce4bc..40adc305a6d 100644 --- a/kiwi/builder/install.py +++ b/kiwi/builder/install.py @@ -161,6 +161,8 @@ def create_install_iso(self) -> None: 'mbr_id': self.mbrid.get_id(), 'application_id': self.xml_state.build_type.get_application_id(), 'efi_mode': self.firmware.efi_mode(), + 'efi_partition_table': self.firmware.get_partition_table_type(), + 'gpt_hybrid_mbr': self.firmware.gpt_hybrid_mbr, 'ofw_mode': self.firmware.ofw_mode(), 'legacy_bios_mode': self.firmware.legacy_bios_mode() } diff --git a/kiwi/builder/live.py b/kiwi/builder/live.py index 509f9016ca3..988701aa434 100644 --- a/kiwi/builder/live.py +++ b/kiwi/builder/live.py @@ -144,6 +144,8 @@ def create(self) -> Result: 'mbr_id': self.mbrid.get_id(), 'application_id': self.application_id, 'efi_mode': self.firmware.efi_mode(), + 'efi_partition_table': self.firmware.get_partition_table_type(), + 'gpt_hybrid_mbr': self.firmware.gpt_hybrid_mbr, 'legacy_bios_mode': self.firmware.legacy_bios_mode() } } diff --git a/kiwi/filesystem/isofs.py b/kiwi/filesystem/isofs.py index 96df8e4395c..a2c14229c08 100644 --- a/kiwi/filesystem/isofs.py +++ b/kiwi/filesystem/isofs.py @@ -50,6 +50,6 @@ def create_on_file( iso_tool.init_iso_creation_parameters(meta_data) if efi_loader: - iso_tool.add_efi_loader_parameters(efi_loader) + iso_tool.add_efi_loader_parameters(efi_loader, meta_data) iso_tool.create_iso(self.filename) diff --git a/kiwi/firmware.py b/kiwi/firmware.py index 8b03f4fae4b..624a21f2cdd 100644 --- a/kiwi/firmware.py +++ b/kiwi/firmware.py @@ -42,6 +42,7 @@ def __init__(self, xml_state): self.firmware = xml_state.build_type.get_firmware() self.efipart_mbytes = xml_state.build_type.get_efipartsize() self.efi_partition_table = xml_state.build_type.get_efiparttable() + self.gpt_hybrid_mbr = xml_state.build_type.get_gpt_hybrid_mbr() self.efi_csm = True if xml_state.build_type.get_eficsm() is None \ else xml_state.build_type.get_eficsm() diff --git a/kiwi/iso_tools/base.py b/kiwi/iso_tools/base.py index 02973a64b2f..fa0cf182812 100644 --- a/kiwi/iso_tools/base.py +++ b/kiwi/iso_tools/base.py @@ -19,7 +19,7 @@ import shutil import logging from typing import ( - Dict, List, Union + Dict, List, Optional, Union ) # project @@ -72,7 +72,9 @@ def init_iso_creation_parameters( """ raise NotImplementedError - def add_efi_loader_parameters(self, loader_file: str) -> None: + def add_efi_loader_parameters( + self, loader_file: str, custom_args: Optional[Dict[str, Union[str, bool]]] = None + ) -> None: """ Add ISO creation parameters to embed the EFI loader diff --git a/kiwi/iso_tools/xorriso.py b/kiwi/iso_tools/xorriso.py index 39eab4f3204..f532584aae6 100644 --- a/kiwi/iso_tools/xorriso.py +++ b/kiwi/iso_tools/xorriso.py @@ -103,6 +103,11 @@ def init_iso_creation_parameters( self.iso_parameters += [ '-compliance', 'untranslated_names' ] + else: + self.iso_parameters += [ + # https://lists.gnu.org/archive/html/bug-xorriso/2024-11/msg00012.html + '-compliance', 'no_emul_toc' + ] if Defaults.is_x86_arch(self.arch) and legacy_bios_mode: mbr_file = os.sep.join( @@ -145,7 +150,9 @@ def init_iso_creation_parameters( '-boot_image', 'any', 'load_size=2048' ] - def add_efi_loader_parameters(self, loader_file: str) -> None: + def add_efi_loader_parameters( + self, loader_file: str, custom_args: Optional[Dict[str, Union[str, bool]]] = None + ) -> None: """ Add ISO creation parameters to embed the EFI loader @@ -156,6 +163,18 @@ def add_efi_loader_parameters(self, loader_file: str) -> None: file refer to _create_embedded_fat_efi_image() from bootloader/config/grub2.py """ + if custom_args: + efi_partition_table = custom_args.get('efi_partition_table', '') + legacy_bios_mode = custom_args.get('legacy_bios_mode', False) + gpt_hybrid_mbr = custom_args.get('gpt_hybrid_mbr', False) + if efi_partition_table == 'gpt': + self.iso_loaders += [ + '-boot_image', 'any', 'appended_part_as=gpt', + ] + if gpt_hybrid_mbr and legacy_bios_mode: + self.iso_loaders += [ + '-boot_image', 'any', 'mbr_force_bootable=on', + ] self.iso_loaders += [ '-append_partition', '2', '0xef', loader_file, '-boot_image', 'any', 'next', @@ -189,3 +208,10 @@ def create_iso( '-chmod', '0755', '/', '--' ] + self.iso_loaders + hidden_files_parameters ) + report_call = Command.run( + [ + self.get_tool_name(), '-indev', filename, + '-report_system_area', 'plain' + ] + ) + log.debug(report_call.output) diff --git a/kiwi/schema/kiwi.rnc b/kiwi/schema/kiwi.rnc index c6a9062b96f..b163fc0512d 100644 --- a/kiwi/schema/kiwi.rnc +++ b/kiwi/schema/kiwi.rnc @@ -1687,7 +1687,7 @@ div { attribute efiparttable { "msdos" | "gpt" } >> sch:pattern [ id = "efiparttable" is-a = "image_type" sch:param [ name = "attr" value = "efiparttable" ] - sch:param [ name = "types" value = "oem" ] + sch:param [ name = "types" value = "oem iso" ] ] k.type.bootprofile.attribute = ## Specifies the boot profile defined in the boot image @@ -2012,7 +2012,7 @@ div { attribute gpt_hybrid_mbr { xsd:boolean } >> sch:pattern [ id = "gpt_hybrid_mbr" is-a = "image_type" sch:param [ name = "attr" value = "gpt_hybrid_mbr" ] - sch:param [ name = "types" value = "oem" ] + sch:param [ name = "types" value = "oem iso" ] ] k.type.initrd_system.attribute = ## specify which initrd builder to use, default is dracut diff --git a/kiwi/schema/kiwi.rng b/kiwi/schema/kiwi.rng index 4ea5fe26277..219e217ca6b 100644 --- a/kiwi/schema/kiwi.rng +++ b/kiwi/schema/kiwi.rng @@ -2473,7 +2473,7 @@ table type. - + @@ -2907,7 +2907,7 @@ create a hybrid GPT/MBR partition table - + diff --git a/test/unit/builder/live_test.py b/test/unit/builder/live_test.py index 0925838a57f..7addf9d8f06 100644 --- a/test/unit/builder/live_test.py +++ b/test/unit/builder/live_test.py @@ -247,6 +247,8 @@ def side_effect(): self.setup.export_package_list.return_value = '.packages' self.firmware.bios_mode.return_value = False + self.firmware.get_partition_table_type.return_value = 'gpt' + self.firmware.gpt_hybrid_mbr = False self.live_image.create() self.setup.import_cdroot_files.assert_called_once_with('temp_media_dir') @@ -369,6 +371,8 @@ def side_effect(): 'volume_id': 'volid', 'efi_mode': 'uefi', 'efi_loader': 'kiwi-tmpfile', + 'efi_partition_table': 'gpt', + 'gpt_hybrid_mbr': False, 'udf': True, 'legacy_bios_mode': True } diff --git a/test/unit/filesystem/isofs_test.py b/test/unit/filesystem/isofs_test.py index 6eef15435a2..b5abdb4aee8 100644 --- a/test/unit/filesystem/isofs_test.py +++ b/test/unit/filesystem/isofs_test.py @@ -58,5 +58,6 @@ def test_create_on_file_EFI_enabled(self, mock_IsoTools): self.isofs.create_on_file('myimage') iso_tool.create_iso.assert_called_once_with('myimage') iso_tool.add_efi_loader_parameters.assert_called_once_with( - 'esp-image-file' + 'esp-image-file', + {'efi_mode': 'uefi', 'efi_loader': 'esp-image-file'} ) diff --git a/test/unit/iso_tools/xorriso_test.py b/test/unit/iso_tools/xorriso_test.py index 9eb5112d718..36e86eb6680 100644 --- a/test/unit/iso_tools/xorriso_test.py +++ b/test/unit/iso_tools/xorriso_test.py @@ -1,5 +1,7 @@ import logging -from unittest.mock import patch +from unittest.mock import ( + patch, call +) from pytest import ( raises, fixture ) @@ -94,7 +96,8 @@ def test_init_iso_creation_parameters_efi(self, mock_os_path_exists): '-preparer_id', 'preparer', '-volid', 'vol_id', '-joliet', 'on', - '-padding', '0' + '-padding', '0', + '-compliance', 'no_emul_toc' ] assert self.iso_tool.iso_loaders == [ '-boot_image', 'grub', @@ -133,7 +136,8 @@ def test_init_iso_creation_parameters_efi_custom_app_id( '-preparer_id', 'preparer', '-volid', 'vol_id', '-joliet', 'on', - '-padding', '0' + '-padding', '0', + '-compliance', 'no_emul_toc' ] def test_add_efi_loader_parameters(self): @@ -147,6 +151,41 @@ def test_add_efi_loader_parameters(self): '-boot_image', 'any', 'emul_type=no_emulation' ] + def test_add_efi_loader_parameters_gpt(self): + self.iso_tool.add_efi_loader_parameters( + 'target_dir/efi-loader', + {'efi_partition_table': 'gpt'} + ) + assert self.iso_tool.iso_loaders == [ + '-boot_image', 'any', 'appended_part_as=gpt', + '-append_partition', '2', '0xef', 'target_dir/efi-loader', + '-boot_image', 'any', 'next', + '-boot_image', 'any', + 'efi_path=--interval:appended_partition_2:all::', + '-boot_image', 'any', 'platform_id=0xef', + '-boot_image', 'any', 'emul_type=no_emulation' + ] + + def test_add_efi_loader_parameters_gpt_hybrid(self): + self.iso_tool.add_efi_loader_parameters( + 'target_dir/efi-loader', + { + 'efi_partition_table': 'gpt', + 'legacy_bios_mode': True, + 'gpt_hybrid_mbr': True + } + ) + assert self.iso_tool.iso_loaders == [ + '-boot_image', 'any', 'appended_part_as=gpt', + '-boot_image', 'any', 'mbr_force_bootable=on', + '-append_partition', '2', '0xef', 'target_dir/efi-loader', + '-boot_image', 'any', 'next', + '-boot_image', 'any', + 'efi_path=--interval:appended_partition_2:all::', + '-boot_image', 'any', 'platform_id=0xef', + '-boot_image', 'any', 'emul_type=no_emulation' + ] + @patch('kiwi.iso_tools.xorriso.Command.run') @patch('kiwi.iso_tools.xorriso.Path.wipe') @patch('kiwi.iso_tools.xorriso.Path.which') @@ -154,15 +193,23 @@ def test_create_iso(self, mock_which, mock_wipe, mock_command): mock_which.return_value = '/usr/bin/xorriso' self.iso_tool.create_iso('myiso', hidden_files=['hide_me']) mock_wipe.assert_called_once_with('myiso') - mock_command.assert_called_once_with( - [ - '/usr/bin/xorriso', - '-outdev', 'myiso', - '-map', 'source-dir', '/', - '-chmod', '0755', '/', '--', - '--', '-find', 'hide_me', '-exec', 'hide', 'on' - ] - ) + assert mock_command.call_args_list == [ + call( + [ + '/usr/bin/xorriso', + '-outdev', 'myiso', + '-map', 'source-dir', '/', + '-chmod', '0755', '/', '--', + '--', '-find', 'hide_me', '-exec', 'hide', 'on' + ] + ), + call( + [ + '/usr/bin/xorriso', '-indev', 'myiso', + '-report_system_area', 'plain' + ] + ) + ] def test_has_iso_hybrid_capability(self): assert self.iso_tool.has_iso_hybrid_capability() is True