| 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
| ||
|
Message-ID: <afec8c0c7b92de59469e789cff4ba8a11f631217.camel@intel.com>
Date: Mon, 2 Sep 2024 06:36:18 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "james.morse@....com" <james.morse@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"rafael@...nel.org" <rafael@...nel.org>, "lenb@...nel.org" <lenb@...nel.org>,
"Ko, Koba" <kobak@...dia.com>, "mochs@...dia.com" <mochs@...dia.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH V2] acpi/prmt: find block with specific type
On Mon, 2024-09-02 at 00:22 +0800, KobaK wrote:
> PRMT needs to find the correct type of block to
> translate the PA-VA mapping for EFI runtime services.
>
> The issue arises because the PRMT is finding a block of
> type EFI_CONVENTIONAL_MEMORY, which is not appropriate for
> runtime services as described in Section 2.2.2 (Runtime
> Services) of the UEFI Specification [1]. Since the PRM handler is
> a type of runtime service, this causes an exception
> when the PRM handler is called.
>
> [Firmware Bug]: Unable to handle paging request in EFI runtime
> service
> WARNING: CPU: 22 PID: 4330 at drivers/firmware/efi/runtime-
> wrappers.c:341
> __efi_queue_work+0x11c/0x170
> Call trace:
> __efi_queue_work+0x11c/0x170
> efi_call_acpi_prm_handler+0x68/0xd0
> acpi_platformrt_space_handler+0x198/0x258
> acpi_ev_address_space_dispatch+0x144/0x388
> acpi_ex_access_region+0x9c/0x118
> acpi_ex_write_serial_bus+0xc4/0x218
> acpi_ex_write_data_to_field+0x168/0x218
> acpi_ex_store_object_to_node+0x1a8/0x258
> acpi_ex_store+0xec/0x330
> acpi_ex_opcode_1A_1T_1R+0x15c/0x618
> acpi_ds_exec_end_op+0x274/0x548
> acpi_ps_parse_loop+0x10c/0x6b8
> acpi_ps_parse_aml+0x140/0x3b0
> acpi_ps_execute_method+0x12c/0x2a0
> acpi_ns_evaluate+0x210/0x310
> acpi_evaluate_object+0x178/0x358
> acpi_proc_write+0x1a8/0x8a0 [acpi_call]
> proc_reg_write+0xcc/0x150
> vfs_write+0xd8/0x380
> ksys_write+0x70/0x120
> __arm64_sys_write+0x24/0x48
> invoke_syscall.constprop.0+0x80/0xf8
> do_el0_svc+0x50/0x110
> el0_svc+0x48/0x1d0
> el0t_64_sync_handler+0x15c/0x178
> el0t_64_sync+0x1a8/0x1b0
>
> Find a block with specific type to fix this.
> prmt find a block with EFI_RUNTIME_SERVICES_DATA for prm handler and
> find a block with EFI_RUNTIME_SERVICES_CODE for prm context.
> If no suitable block is found, a warning message will be prompted
> but the procedue continues to manage the next prm handler.
> However, if the prm handler is actullay called without proper
> allocation,
> it would result in a failure during error handling.
>
> By using the correct memory types for runtime services,
> Ensure that the PRM handler and the context are
> properly mapped in the virtual address space during runtime,
> preventing the paging request error.
>
> [1]
> https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf
>
> Fixes: cefc7ca46235 ("ACPI: PRM: implement OperationRegion handler
> for the PlatformRtMechanism subtype")
> Signed-off-by: KobaK <kobak@...dia.com>
> Reviewed-by: Matthew R. Ochs <mochs@...dia.com>
> ---
> V2:
> 1. format the changelog and add more about error handling.
> 2. replace goto
> ---
> drivers/acpi/prmt.c | 47 ++++++++++++++++++++++++++++++-------------
> --
> 1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> index c78453c74ef5..281cdb53eddb 100644
> --- a/drivers/acpi/prmt.c
> +++ b/drivers/acpi/prmt.c
> @@ -72,17 +72,21 @@ struct prm_module_info {
> struct prm_handler_info handlers[]
> __counted_by(handler_count);
> };
>
> -static u64 efi_pa_va_lookup(u64 pa)
> +static u64 efi_pa_va_lookup(u64 pa, u32 type)
> {
> efi_memory_desc_t *md;
> u64 pa_offset = pa & ~PAGE_MASK;
> u64 page = pa & PAGE_MASK;
>
> for_each_efi_memory_desc(md) {
> - if (md->phys_addr < pa && pa < md->phys_addr +
> PAGE_SIZE * md->num_pages)
> + if ((md->type == type) &&
> + (md->phys_addr < pa && pa < md->phys_addr +
> PAGE_SIZE * md->num_pages)) {
> return pa_offset + md->virt_addr + page - md-
> >phys_addr;
> + }
> }
>
> + pr_warn("PRM: Failed to find a VA block for pa: %lx type
> %u\n", pa, type);
> +
> return 0;
> }
>
> @@ -148,9 +152,12 @@ acpi_parse_prmt(union acpi_subtable_headers
> *header, const unsigned long end)
> th = &tm->handlers[cur_handler];
>
> guid_copy(&th->guid, (guid_t *)handler_info-
> >handler_guid);
> - th->handler_addr = (void
> *)efi_pa_va_lookup(handler_info->handler_address);
> - th->static_data_buffer_addr =
> efi_pa_va_lookup(handler_info->static_data_buffer_address);
> - th->acpi_param_buffer_addr =
> efi_pa_va_lookup(handler_info->acpi_param_buffer_address);
> + th->handler_addr =
> + (void *)efi_pa_va_lookup(handler_info-
> >handler_address, EFI_RUNTIME_SERVICES_CODE);
> + th->static_data_buffer_addr =
> + efi_pa_va_lookup(handler_info-
> >static_data_buffer_address, EFI_RUNTIME_SERVICES_DATA);
> + th->acpi_param_buffer_addr =
> + efi_pa_va_lookup(handler_info-
> >acpi_param_buffer_address, EFI_RUNTIME_SERVICES_DATA);
why not remove pr_warn() in efi_pa_va_lookup(), and check for all three
addrs here with a more detailed message about which part is missing for
which module/handler?
> } while (++cur_handler < tm->handler_count && (handler_info =
> get_next_handler(handler_info)));
>
> return 0;
> @@ -250,8 +257,18 @@ static acpi_status
> acpi_platformrt_space_handler(u32 function,
>
> handler = find_prm_handler(&buffer->handler_guid);
> module = find_prm_module(&buffer->handler_guid);
> - if (!handler || !module)
> - goto invalid_guid;
> + if (!handler || !module) {
> + buffer->prm_status =
> PRM_HANDLER_GUID_NOT_FOUND;
> + pr_err("PRM: PRM Handler GUID is not
> found\n");
Are you sure you want to get this error message every time the address
space handler is invoked, rather than give an one time warning during
the handler probe time?
thanks,
rui
> + return AE_OK;
> + }
> +
> + if (!handler->handler_addr || !handler-
> >static_data_buffer_addr ||
> + !handler->acpi_param_buffer_addr) {
> + buffer->prm_status = PRM_HANDLER_ERROR;
> + pr_err("PRM: PRM Handler not found\n");
> + return AE_OK;
> + }
>
> ACPI_COPY_NAMESEG(context.signature, "PRMC");
> context.revision = 0x0;
> @@ -274,8 +291,10 @@ static acpi_status
> acpi_platformrt_space_handler(u32 function,
> case PRM_CMD_START_TRANSACTION:
>
> module = find_prm_module(&buffer->handler_guid);
> - if (!module)
> - goto invalid_guid;
> + if (!module) {
> + buffer->prm_status =
> PRM_HANDLER_GUID_NOT_FOUND;
> + return AE_OK;
> + }
>
> if (module->updatable)
> module->updatable = false;
> @@ -286,8 +305,10 @@ static acpi_status
> acpi_platformrt_space_handler(u32 function,
> case PRM_CMD_END_TRANSACTION:
>
> module = find_prm_module(&buffer->handler_guid);
> - if (!module)
> - goto invalid_guid;
> + if (!module) {
> + buffer->prm_status =
> PRM_HANDLER_GUID_NOT_FOUND;
> + return AE_OK;
> + }
>
> if (module->updatable)
> buffer->prm_status =
> UPDATE_UNLOCK_WITHOUT_LOCK;
> @@ -302,10 +323,6 @@ static acpi_status
> acpi_platformrt_space_handler(u32 function,
> }
>
> return AE_OK;
> -
> -invalid_guid:
> - buffer->prm_status = PRM_HANDLER_GUID_NOT_FOUND;
> - return AE_OK;
> }
>
> void __init init_prmt(void)
Powered by blists - more mailing lists