| 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: <20200221095223.GC29220@e121166-lin.cambridge.arm.com>
Date: Fri, 21 Feb 2020 09:52:23 +0000
From: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Pankaj Bansal <pankaj.bansal@....com>,
Hanjun Guo <guohanjun@...wei.com>,
Will Deacon <will@...nel.org>,
Sudeep Holla <sudeep.holla@....com>,
Catalin Marinas <catalin.marinas@....com>,
Robin Murphy <robin.murphy@....com>,
Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 137/191] ACPI/IORT: Fix Number of IDs handling in
iort_id_map()
On Fri, Feb 21, 2020 at 08:41:50AM +0100, Greg Kroah-Hartman wrote:
> From: Hanjun Guo <guohanjun@...wei.com>
>
> [ Upstream commit 3c23b83a88d00383e1d498cfa515249aa2fe0238 ]
Please drop this patch from the stable queue, thanks.
Lorenzo
> The IORT specification [0] (Section 3, table 4, page 9) defines the
> 'Number of IDs' as 'The number of IDs in the range minus one'.
>
> However, the IORT ID mapping function iort_id_map() treats the 'Number
> of IDs' field as if it were the full IDs mapping count, with the
> following check in place to detect out of boundary input IDs:
>
> InputID >= Input base + Number of IDs
>
> This check is flawed in that it considers the 'Number of IDs' field as
> the full number of IDs mapping and disregards the 'minus one' from
> the IDs count.
>
> The correct check in iort_id_map() should be implemented as:
>
> InputID > Input base + Number of IDs
>
> this implements the specification correctly but unfortunately it breaks
> existing firmwares that erroneously set the 'Number of IDs' as the full
> IDs mapping count rather than IDs mapping count minus one.
>
> e.g.
>
> PCI hostbridge mapping entry 1:
> Input base: 0x1000
> ID Count: 0x100
> Output base: 0x1000
> Output reference: 0xC4 //ITS reference
>
> PCI hostbridge mapping entry 2:
> Input base: 0x1100
> ID Count: 0x100
> Output base: 0x2000
> Output reference: 0xD4 //ITS reference
>
> Two mapping entries which the second entry's Input base = the first
> entry's Input base + ID count, so for InputID 0x1100 and with the
> correct InputID check in place in iort_id_map() the kernel would map
> the InputID to ITS 0xC4 not 0xD4 as it would be expected.
>
> Therefore, to keep supporting existing flawed firmwares, introduce a
> workaround that instructs the kernel to use the old InputID range check
> logic in iort_id_map(), so that we can support both firmwares written
> with the flawed 'Number of IDs' logic and the correct one as defined in
> the specifications.
>
> [0]: http://infocenter.arm.com/help/topic/com.arm.doc.den0049d/DEN0049D_IO_Remapping_Table.pdf
>
> Reported-by: Pankaj Bansal <pankaj.bansal@....com>
> Link: https://lore.kernel.org/linux-acpi/20191215203303.29811-1-pankaj.bansal@nxp.com/
> Signed-off-by: Hanjun Guo <guohanjun@...wei.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> Cc: Pankaj Bansal <pankaj.bansal@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Sudeep Holla <sudeep.holla@....com>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Will Deacon <will@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
> drivers/acpi/arm64/iort.c | 57 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index e11b5da6f828f..7d86468300b78 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -306,6 +306,59 @@ out:
> return status;
> }
>
> +struct iort_workaround_oem_info {
> + char oem_id[ACPI_OEM_ID_SIZE + 1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> +};
> +
> +static bool apply_id_count_workaround;
> +
> +static struct iort_workaround_oem_info wa_info[] __initdata = {
> + {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP07 ",
> + .oem_revision = 0,
> + }, {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP08 ",
> + .oem_revision = 0,
> + }
> +};
> +
> +static void __init
> +iort_check_id_count_workaround(struct acpi_table_header *tbl)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(wa_info); i++) {
> + if (!memcmp(wa_info[i].oem_id, tbl->oem_id, ACPI_OEM_ID_SIZE) &&
> + !memcmp(wa_info[i].oem_table_id, tbl->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
> + wa_info[i].oem_revision == tbl->oem_revision) {
> + apply_id_count_workaround = true;
> + pr_warn(FW_BUG "ID count for ID mapping entry is wrong, applying workaround\n");
> + break;
> + }
> + }
> +}
> +
> +static inline u32 iort_get_map_max(struct acpi_iort_id_mapping *map)
> +{
> + u32 map_max = map->input_base + map->id_count;
> +
> + /*
> + * The IORT specification revision D (Section 3, table 4, page 9) says
> + * Number of IDs = The number of IDs in the range minus one, but the
> + * IORT code ignored the "minus one", and some firmware did that too,
> + * so apply a workaround here to keep compatible with both the spec
> + * compliant and non-spec compliant firmwares.
> + */
> + if (apply_id_count_workaround)
> + map_max--;
> +
> + return map_max;
> +}
> +
> static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
> u32 *rid_out)
> {
> @@ -322,8 +375,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
> return -ENXIO;
> }
>
> - if (rid_in < map->input_base ||
> - (rid_in >= map->input_base + map->id_count))
> + if (rid_in < map->input_base || rid_in > iort_get_map_max(map))
> return -ENXIO;
>
> *rid_out = map->output_base + (rid_in - map->input_base);
> @@ -1542,5 +1594,6 @@ void __init acpi_iort_init(void)
> return;
> }
>
> + iort_check_id_count_workaround(iort_table);
> iort_init_platform_devices();
> }
> --
> 2.20.1
>
>
>
Powered by blists - more mailing lists