linux-kernel - Re: [PATCH v2 2/3] arch/x86: Add <asm/ima-efi.h> for arch_ima_efi_boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 3 Jan 2024 14:45:28 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 bhelgaas@...gle.com, arnd@...db.de, zohar@...ux.ibm.com,
 dmitry.kasatkin@...il.com, paul@...l-moore.com, jmorris@...ei.org,
 serge@...lyn.com, javierm@...hat.com, linux-arch@...r.kernel.org,
 linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org, linux-integrity@...r.kernel.org,
 linux-security-module@...r.kernel.org
Subject: Re: [PATCH v2 2/3] arch/x86: Add <asm/ima-efi.h> for
 arch_ima_efi_boot_mode

Hi

Am 03.01.24 um 14:11 schrieb Ard Biesheuvel:
> On Tue, 2 Jan 2024 at 15:07, Thomas Zimmermann <tzimmermann@...e.de> wrote:
>>
>> Hii Ard
>>
>> Am 19.12.23 um 12:38 schrieb Ard Biesheuvel:
>>> Hi Thomas,
>>>
>>> On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <tzimmermann@...e.de> wrote:
>>>>
>>>> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
>>>> which expands to use struct boot_params from <asm/bootparams.h>. Many
>>>> drivers include <linux/efi.h>, but do not use boot parameters. Changes
>>>> to bootparam.h or its included headers can easily trigger large,
>>>> unnessary rebuilds of the kernel.
>>>>
>>>> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
>>>> <asm/setup.h> separates that dependency from the rest of the EFI
>>>> interfaces. The only user is in ima_efi.c. As the file already declares
>>>> a default value for arch_ima_efi_boot_mode, move this define into
>>>> asm-generic for all other architectures.
>>>>
>>>> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
>>>> later be removed from further x86 header files.
>>>>
>>>
>>> Apologies if I missed this in v1 but is the new asm-generic header
>>> really necessary? Could we instead turn arch_ima_efi_boot_mode into a
>>> function that is a static inline { return unset; } by default, but can
>>> be emitted out of line in one of the x86/platform/efi.c source files,
>>> where referring to boot_params is fine?
>>
>> I cannot figure out how to do this without *something* in asm-generic or
>> adding if-CONFIG_X86 guards in ima-efi.c.
>>
>> But I noticed that linux/efi.h already contains 2 or 3 ifdef branches
>> for x86. Would it be an option to move this code into asm/efi.h
>> (including a header file in asm-generic for the non-x86 variants) and
>> add the arch_ima_efi_boot_mode() helper there as well?  At least that
>> wouldn't be a header for only a single define.
>>
> 
> Could we just move the x86 implementation out of line?
> 
> So something like this in arch/x86/include/asm/efi.h
> 
> enum efi_secureboot_mode x86_ima_efi_boot_mode(void);
> #define arch_ima_efi_boot_mode x86_ima_efi_boot_mode()
> 
> and an implementation in one of the related .c files:
> 
> enum efi_secureboot_mode x86_ima_efi_boot_mode(void)
> {
>      return boot_params.secure_boot;
> }
> 
> ?

Well, that's just enough to avoid boot_params within the header file. 
But it should work.

Best regards
Thomas


-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (841 bytes)