linux-kernel - Re: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

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: <24a741a4-b305-c817-e8c3-34b213ad0ee5@intel.com>
Date:   Thu, 7 Sep 2023 15:19:48 +0200
From:   "Wilczynski, Michal" <michal.wilczynski@...el.com>
To:     Jason Andryuk <jandryuk@...il.com>,
        Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>
CC:     Roger Pau Monne <roger.pau@...rix.com>, <stable@...r.kernel.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "Borislav Petkov" <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, <xen-devel@...ts.xenproject.org>,
        <linux-kernel@...r.kernel.org>, <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running
 as Xen dom0


Hi,

On 9/6/2023 8:21 PM, Jason Andryuk wrote:
> From: Roger Pau Monne <roger.pau@...rix.com>
>
> The Processor _PDC buffer bits notify ACPI of the OS capabilities, and
> so ACPI can adjust the return of other Processor methods taking the OS
> capabilities into account.

_PDC method is deprecated for this purpose, since 2018, and is dropped from
spec since 6.5

We made the switch in linux since 6.6:
95272641338a ("ACPI: processor: Use _OSC to convey OSPM processor support information")

>
> When Linux is running as a Xen dom0, it's the hypervisor the entity
> in charge of processor power management, and hence Xen needs to make
> sure the capabilities reported in the _PDC buffer match the
> capabilities of the driver in Xen.

So I guess you would need to sanitize buffer passed to _OSC method instead ?

>
> Introduce a small helper to sanitize the buffer when running as Xen
> dom0.
>
> When Xen supports HWP, this serves as the equivalent of commit
> a21211672c9a ("ACPI / processor: Request native thermal interrupt
> handling via _OSC") to avoid SMM crashes.  Xen will set bit 12 in the
> _PDC bits and the _PDC call will apply it.
>
> [ jandryuk: Mention Xen HWP's need.  Move local variables ]
> Signed-off-by: Roger Pau Monné <roger.pau@...rix.com>
> Cc: stable@...r.kernel.org
> Signed-off-by: Jason Andryuk <jandryuk@...il.com>
> ---
> v2:
> Move local variables in acpi_processor_eval_pdc() to reuse in both conditions.
> ---
>  arch/x86/include/asm/xen/hypervisor.h |  6 ++++++
>  arch/x86/xen/enlighten.c              | 19 +++++++++++++++++++
>  drivers/acpi/processor_pdc.c          | 22 ++++++++++++++++------
>  3 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
> index 5fc35f889cd1..0f88a7e450d3 100644
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -63,4 +63,10 @@ void __init xen_pvh_init(struct boot_params *boot_params);
>  void __init mem_map_via_hcall(struct boot_params *boot_params_p);
>  #endif
>  
> +#ifdef CONFIG_XEN_DOM0
> +void xen_sanitize_pdc(uint32_t *buf);
> +#else
> +static inline void xen_sanitize_pdc(uint32_t *buf) { BUG(); }
> +#endif
> +
>  #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index b8db2148c07d..9f7fc11330a3 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -346,3 +346,22 @@ void xen_arch_unregister_cpu(int num)
>  }
>  EXPORT_SYMBOL(xen_arch_unregister_cpu);
>  #endif
> +
> +#ifdef CONFIG_XEN_DOM0
> +void xen_sanitize_pdc(uint32_t *buf)
> +{
> +	struct xen_platform_op op = {
> +		.cmd			= XENPF_set_processor_pminfo,
> +		.interface_version	= XENPF_INTERFACE_VERSION,
> +		.u.set_pminfo.id	= -1,
> +		.u.set_pminfo.type	= XEN_PM_PDC,
> +	};
> +	int ret;
> +
> +	set_xen_guest_handle(op.u.set_pminfo.pdc, buf);
> +	ret = HYPERVISOR_platform_op(&op);
> +	if (ret)
> +		pr_info("sanitize of _PDC buffer bits from Xen failed: %d\n",
> +		        ret);
> +}
> +#endif
> diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
> index 18fb04523f93..9393dd4a3158 100644
> --- a/drivers/acpi/processor_pdc.c
> +++ b/drivers/acpi/processor_pdc.c
> @@ -122,6 +122,11 @@ static acpi_status
>  acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
>  {
>  	acpi_status status = AE_OK;
> +	union acpi_object *obj;
> +	u32 *buffer = NULL;
> +
> +	obj = pdc_in->pointer;
> +	buffer = (u32 *)(obj->buffer.pointer);
>  
>  	if (boot_option_idle_override == IDLE_NOMWAIT) {
>  		/*
> @@ -129,14 +134,19 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
>  		 * mode will be disabled in the parameter of _PDC object.
>  		 * Of course C1_FFH access mode will also be disabled.
>  		 */
> -		union acpi_object *obj;
> -		u32 *buffer = NULL;
> -
> -		obj = pdc_in->pointer;
> -		buffer = (u32 *)(obj->buffer.pointer);
>  		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
> -
>  	}
> +
> +	if (xen_initial_domain()) {
> +		/*
> +		 * When Linux is running as Xen dom0, the hypervisor is the
> +		 * entity in charge of the processor power management, and so
> +		 * Xen needs to check the OS capabilities reported in the _PDC
> +		 * buffer matches what the hypervisor driver supports.
> +		 */
> +		xen_sanitize_pdc(buffer);
> +	}
> +
>  	status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
>  
>  	if (ACPI_FAILURE(status))