linux-kernel - Re: [PATCH 1/2] ARM/efi: Remove duplicate permission settings

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXHpJ3gz=+Keswx0MZ8v6YQENR2pjeS_CE6g4cXML2LQhA@mail.gmail.com>
Date: Thu, 30 Oct 2025 11:36:13 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Qiang Ma <maqianga@...ontech.com>
Cc: linux@...linux.org.uk, linux-efi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] ARM/efi: Remove duplicate permission settings

On Thu, 30 Oct 2025 at 11:25, Qiang Ma <maqianga@...ontech.com> wrote:
>
>
> 在 2025/10/30 18:02, Ard Biesheuvel 写道:
> > On Thu, 30 Oct 2025 at 08:37, Qiang Ma <maqianga@...ontech.com> wrote:
> >>
> >> 在 2025/10/29 22:15, Ard Biesheuvel 写道:
> >>> On Wed, 29 Oct 2025 at 10:55, Qiang Ma <maqianga@...ontech.com> wrote:
> >>>> 在 2025/10/28 21:42, Ard Biesheuvel 写道:
> >>>>> On Mon, 27 Oct 2025 at 04:46, Qiang Ma <maqianga@...ontech.com> wrote:
> >>>>>> 在 2025/10/23 16:30, Ard Biesheuvel 写道:
> >>>>>>> On Thu, 23 Oct 2025 at 10:22, Qiang Ma <maqianga@...ontech.com> wrote:
> >>>>>>>> In the efi_virtmap_init(), permission settings have been applied:
> >>>>>>>>
> >>>>>>>> static bool __init efi_virtmap_init(void)
> >>>>>>>> {
> >>>>>>>>             ...
> >>>>>>>>             for_each_efi_memory_desc(md)
> >>>>>>>>                     ...
> >>>>>>>>                     efi_create_mapping(&efi_mm, md);
> >>>>>>>>             ...
> >>>>>>>>             efi_memattr_apply_permissions(&efi_mm, efi_set_mapping_permissions);
> >>>>>>>>             ...
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> Therefore, there is no need to apply it again in the efi_create_mapping().
> >>>>>>>>
> >>>>>>>> Fixes: 9fc68b717c24 ("ARM/efi: Apply strict permissions for UEFI Runtime Services regions")
> >>>>>>>>
> >>>>>>>> Signed-off-by: Qiang Ma <maqianga@...ontech.com>
> >>>>>>> No, efi_memattr_apply_permissions() uses the /optional/ memory
> >>>>>>> attributes table, whereas efi_create_mapping() uses the permission
> >>>>>>> attributes in the EFI memory map. The memory attributes table is
> >>>>>>> optional, in which case any RO/XP attributes from the memory map
> >>>>>>> should be used.
> >>>>>>>
> >>>>>> I see.
> >>>>>>
> >>>>>> Then, can it be modified like this?
> >>>>> No
> >>>>>
> >>>>>> --- a/arch/arm/kernel/efi.c
> >>>>>> +++ b/arch/arm/kernel/efi.c
> >>>>>> @@ -65,16 +65,13 @@ int __init efi_create_mapping(struct mm_struct *mm,
> >>>>>> efi_memory_desc_t *md)
> >>>>>>                     desc.type = MT_MEMORY_RWX_NONCACHED;
> >>>>>>             else if (md->attribute & EFI_MEMORY_WC)
> >>>>>>                     desc.type = MT_DEVICE_WC;
> >>>>>> +       else if (md->attribute & (EFI_MEMORY_RO | EFI_MEMORY_XP))
> >>>>> This will be true for RO, XP or RO+XP.
> >>>>>
> >>>>>> +               desc.type = MT_MEMORY_RO;
> >>>>> This will apply RO permissions even to XP regions, which need to be writable.
> >>>>>
> >>>> Thanks for your review.
> >>>> I see.
> >>>>
> >>>> I can introduce a new type MT_MEMORY_RO_XP, to describe RO+XP,
> >>>> and then we can use the RO+XP attribute to implement memory mapping.
> >>>>
> >>> Why? The current code is working fine, no?
> >>>
> >> Yes, the current code is running normally.
> >>
> >> The reasons for the modification are as follows:
> >> I noticed that the arm64/RISC-V efi_create_mapping() always return 0,
> >> but in the code where efi_virtmap_init() calls it, it is as follows:
> >>
> >> ret = efi_create_mapping(&efi_mm, md);
> >> if (ret) {
> >>       pr_warn("  EFI remap %pa: failed to create mapping (%d)\n",
> >>           &phys, ret);
> >>       return false;
> >> }
> >>
> >> This return error print is unnecessary, so I want to remove it.
> > So what is preventing you from removing this from the RISC-V version?
> >
> The current idea is to first remove the unnecessary return print from
> arm/arm64,

Please leave the ARM code alone.

> and then remove RISC-V later, as this RISC-V code is also adapted based
> on arm64.
>

RISC-V copied the ARM code and used it as a starting point. That does
not mean it has to remain that way.