linux-kernel - Re: [PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC

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: <559AF116.4030208@redhat.com>
Date:	Mon, 06 Jul 2015 15:20:22 -0600
From:	Al Stone <ahs3@...hat.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
CC:	Catalin Marinas <catalin.marinas@....com>,
	Al Stone <al.stone@...aro.org>, linux-acpi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linaro-kernel@...ts.linaro.org, jason@...edaemon.net,
	linaro-acpi@...ts.linaro.org, patches@...aro.org,
	linux-kernel@...r.kernel.org, will.deacon@....com,
	tglx@...utronix.de, lenb@...nel.org
Subject: Re: [PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro

On 07/03/2015 05:54 PM, Rafael J. Wysocki wrote:
> On Friday, July 03, 2015 01:51:36 PM Al Stone wrote:
>> On 07/03/2015 08:06 AM, Catalin Marinas wrote:
>>> On Thu, Jul 02, 2015 at 05:48:35PM -0600, Al Stone wrote:
>>>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>>>> index 39248d3..a3c26a4 100644
>>>> --- a/arch/arm64/include/asm/acpi.h
>>>> +++ b/arch/arm64/include/asm/acpi.h
>>>> @@ -19,6 +19,17 @@
>>>>  #include <asm/psci.h>
>>>>  #include <asm/smp_plat.h>
>>>>  
>>>> +/* Macros for consistency checks of the GICC subtable of MADT */
>>>> +#define ACPI_MADT_GICC_51_LENGTH       76
>>>> +#define ACPI_MADT_GICC_60_LENGTH       80
>>>> +
>>>> +#define BAD_MADT_GICC_ENTRY(entry, end) (                                   \
>>>> +		(!entry) || (unsigned long)entry + sizeof(*entry) > end ||  \
>>>> +		((ACPI_FADT_SPEC_VERSION == ACPI_FADT_SPEC_VERSION_51) &&   \
>>>> +		 (entry->header.length != ACPI_MADT_GICC_51_LENGTH))    ||  \
>>>> +		((ACPI_FADT_SPEC_VERSION == ACPI_FADT_SPEC_VERSION_60) &&   \
>>>> +		 (entry->header.length != ACPI_MADT_GICC_60_LENGTH)))
>>>
>>> This looks ugly but, well, we could live with this.
>>
>> Nod.  It's right at the hairy edge of becoming a function, I think.
>>
>>> However, I'd like to avoid having to extend this macro every time we get
>>> a new spec released, like 6.1 defining another 80 or 84 etc. So, how
>>> about we only update this when there is an actual change in the length?
>>> Something like:
>>>
>>> #define ACPI_MADT_GICC_LENGTH	({					\
>>> 	u8 length;							\
>>> 	if (ACPI_FADT_SPEC_VERSION < ACPI_FADT_SPEC_VERSION_6_0)	\
>>> 		length = 76;						\
>>> 	else								\
>>> 		length = 80;						\
>>> 	length;								\
>>> })
>>>
>>> or just:
>>>
>>> #define ACPI_MADT_GICC_LENGTH	\
>>> 	(ACPI_FADT_SPEC_VERSION < ACPI_FADT_SPEC_VERSION_6_0 ? 76 : 80)
>>>
>>> (the latter is simpler but may not look nice if we change it again in
>>> 6.1; though we could re-write this macro when needed, not a problem)
>>>
>>
>> Perhaps the sanity checking for the MADT subtables needs to be revisited
>> and a more general solution provided -- this is not the only MADT subtable
>> with this problem and it may occur again.
>>
>> Even the versions above are not technically compliant with the spec.  If
>> we implement what the spec currently says, it might look something like
>> this:
>>
>> #define ACPI_MADT_GICC_LENGTH ({					\
>> 	u8 length;							\
>> 	switch (ACPI_FADT_SPEC_VERSION) {				\
>> 	case ACPI_FADT_SPEC_VERSION_5_0:				\
>> 		length = 40;						\
>> 		break;							\
>> 	case ACPI_FADT_SPEC_VERSION_5_1:				\
>> 		length = 76;						\
>> 		break;							\
>> 	default:	/* use 6.0 size */				\
>> 		length = 80;						\
>> 	}								\
>> 	length;								\
>> })
>>
>> So it's just messy and there will be a need for change.  Let me think about
>> making this a function instead of a macro; it may make sense to really fix
>> BAD_MADT_ENTRY in general instead of just dealing with the GICC subtable,
>> but it could also be overkill.
> 
> So here's my suggestion.
> 
> First, make ARM64 boot with 4.2+ in the simplest way possible.

ACK.

> Second, set out to fix BAD_MADT_ENTRY() etc.  Start with fixing ACPICA to
> distinguish between the different formats depending on the spec version and
> follow up from there.
> 
> Thanks,
> Rafael

Yup, that's what I was thinking.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@...hat.com
-----------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/