[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMj1kXHu5ABgxKsc_gg1j=pWMz6DbWoqv=qAAjx-5CiSF2PAiQ@mail.gmail.com>
Date: Wed, 29 Oct 2025 15:15:59 +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 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?
Powered by blists - more mailing lists