linux-kernel - Re: [RFC PATCH v4 13/28] efi: Update efi_mem

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: <41d5df05-14be-ff33-a7e2-6b2f51e2605a@amd.com>
Date:   Thu, 23 Feb 2017 11:27:55 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Matt Fleming <matt@...eblueprint.co.uk>
CC:     <linux-arch@...r.kernel.org>, <linux-efi@...r.kernel.org>,
        <kvm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        <kasan-dev@...glegroups.com>, <linux-mm@...ck.org>,
        <iommu@...ts.linux-foundation.org>, Rik van Riel <riel@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Toshimitsu Kani <toshi.kani@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Corbet <corbet@....net>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Larry Woodman <lwoodman@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [RFC PATCH v4 13/28] efi: Update efi_mem_type() to return defined
 EFI mem types

On 2/21/2017 6:05 AM, Matt Fleming wrote:
> On Thu, 16 Feb, at 09:44:57AM, Tom Lendacky wrote:
>> Update the efi_mem_type() to return EFI_RESERVED_TYPE instead of a
>> hardcoded 0.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>>  arch/x86/platform/efi/efi.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
>> index a15cf81..6407103 100644
>> --- a/arch/x86/platform/efi/efi.c
>> +++ b/arch/x86/platform/efi/efi.c
>> @@ -1037,7 +1037,7 @@ u32 efi_mem_type(unsigned long phys_addr)
>>  	efi_memory_desc_t *md;
>>
>>  	if (!efi_enabled(EFI_MEMMAP))
>> -		return 0;
>> +		return EFI_RESERVED_TYPE;
>>
>>  	for_each_efi_memory_desc(md) {
>>  		if ((md->phys_addr <= phys_addr) &&
>> @@ -1045,7 +1045,7 @@ u32 efi_mem_type(unsigned long phys_addr)
>>  				  (md->num_pages << EFI_PAGE_SHIFT))))
>>  			return md->type;
>>  	}
>> -	return 0;
>> +	return EFI_RESERVED_TYPE;
>>  }
>
> I see what you're getting at here, but arguably the return value in
> these cases never should have been zero to begin with (your change
> just makes that more obvious).
>
> Returning EFI_RESERVED_TYPE implies an EFI memmap entry exists for
> this address, which is misleading because it doesn't in the hunks
> you've modified above.
>
> Instead, could you look at returning a negative error value in the
> usual way we do in the Linux kernel, and update the function prototype
> to match? I don't think any callers actually require the return type
> to be u32.

I can do that, I'll change the return type to an int. For the
!efi_enabled I can return -ENOTSUPP and for when an entry isn't
found I can return -EINVAL.  Sound good?

The ia64 arch is the only other arch that defines the function. It
has just a single return 0 that I'll change to -EINVAL.

Thanks,
Tom

>