linux-kernel - Re: [PATCH] iommu/vt-d: Check VT-d RMRR region in BIOS is reported as reserved

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]

Date:   Wed, 16 Oct 2019 08:25:17 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Yian Chen <yian.chen@...el.com>, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, linux-ia64@...r.kernel.org,
        David Woodhouse <dwmw2@...radead.org>,
        Joerg Roedel <joro@...tes.org>,
        Ashok Raj <ashok.raj@...el.com>,
        Sohil Mehta <sohil.mehta@...el.com>,
        Tony Luck <tony.luck@...el.com>
Cc:     baolu.lu@...ux.intel.com
Subject: Re: [PATCH] iommu/vt-d: Check VT-d RMRR region in BIOS is reported as
 reserved

Hi,

On 10/16/19 12:49 AM, Yian Chen wrote:
> VT-d RMRR (Reserved Memory Region Reporting) regions are reserved
> for device use only and should not be part of allocable memory pool of OS.
> 
> BIOS e820_table reports complete memory map to OS, including OS usable
> memory ranges and BIOS reserved memory ranges etc.
> 
> x86 BIOS may not be trusted to include RMRR regions as reserved type
> of memory in its e820 memory map, hence validate every RMRR entry
> with the e820 memory map to make sure the RMRR regions will not be
> used by OS for any other purposes.
> 
> ia64 EFI is working fine so implement RMRR validation as a dummy function
> 
> Reviewed-by: Sohil Mehta <sohil.mehta@...el.com>
> Signed-off-by: Yian Chen <yian.chen@...el.com>

This patch looks good to me.

Reviewed-by: Lu Baolu <baolu.lu@...ux.intel.com>

> ---
>   arch/ia64/include/asm/iommu.h |  5 +++++
>   arch/x86/include/asm/iommu.h  | 18 ++++++++++++++++++
>   drivers/iommu/intel-iommu.c   |  8 +++++++-
>   3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/ia64/include/asm/iommu.h b/arch/ia64/include/asm/iommu.h
> index 7904f591a79b..eb0db20c9d4c 100644
> --- a/arch/ia64/include/asm/iommu.h
> +++ b/arch/ia64/include/asm/iommu.h
> @@ -2,6 +2,8 @@
>   #ifndef _ASM_IA64_IOMMU_H
>   #define _ASM_IA64_IOMMU_H 1
>   
> +#include <linux/acpi.h>
> +
>   /* 10 seconds */
>   #define DMAR_OPERATION_TIMEOUT (((cycles_t) local_cpu_data->itc_freq)*10)
>   
> @@ -9,6 +11,9 @@ extern void no_iommu_init(void);
>   #ifdef	CONFIG_INTEL_IOMMU
>   extern int force_iommu, no_iommu;
>   extern int iommu_detected;
> +
> +static inline int __init
> +arch_rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr) { return 0; }
>   #else
>   #define no_iommu		(1)
>   #define iommu_detected		(0)
> diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
> index b91623d521d9..95fa65a5f0dc 100644
> --- a/arch/x86/include/asm/iommu.h
> +++ b/arch/x86/include/asm/iommu.h
> @@ -2,10 +2,28 @@
>   #ifndef _ASM_X86_IOMMU_H
>   #define _ASM_X86_IOMMU_H
>   
> +#include <linux/acpi.h>
> +
> +#include <asm/e820/api.h>
> +
>   extern int force_iommu, no_iommu;
>   extern int iommu_detected;
>   
>   /* 10 seconds */
>   #define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000)
>   
> +static inline int __init
> +arch_rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
> +{
> +	u64 start = rmrr->base_address;
> +	u64 end = rmrr->end_address + 1;
> +
> +	if (e820__mapped_all(start, end, E820_TYPE_RESERVED))
> +		return 0;
> +
> +	pr_err(FW_BUG "No firmware reserved region can cover this RMRR [%#018Lx-%#018Lx], contact BIOS vendor for fixes\n",
> +	       start, end - 1);
> +	return -EFAULT;
> +}
> +
>   #endif /* _ASM_X86_IOMMU_H */
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3f974919d3bd..722290014143 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4306,13 +4306,19 @@ int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg)
>   {
>   	struct acpi_dmar_reserved_memory *rmrr;
>   	struct dmar_rmrr_unit *rmrru;
> +	int ret;
> +
> +	rmrr = (struct acpi_dmar_reserved_memory *)header;
> +	ret = arch_rmrr_sanity_check(rmrr);
> +	if (ret)
> +		return ret;
>   
>   	rmrru = kzalloc(sizeof(*rmrru), GFP_KERNEL);
>   	if (!rmrru)
>   		goto out;
>   
>   	rmrru->hdr = header;
> -	rmrr = (struct acpi_dmar_reserved_memory *)header;
> +
>   	rmrru->base_address = rmrr->base_address;
>   	rmrru->end_address = rmrr->end_address;
>   
>