linux-kernel - Re: [RFC][PATCH] arm64: Add support for CONFIG_DEBUG

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] [day] [month] [year] [list]

Message-ID: <58b065d6-839c-7ec0-5594-741a72851edb@redhat.com>
Date:   Wed, 2 Nov 2016 14:28:02 -0600
From:   Laura Abbott <labbott@...hat.com>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     Mark Rutland <mark.rutland@....com>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] arm64: Add support for CONFIG_DEBUG_VIRTUAL

On 10/29/2016 02:39 AM, Ard Biesheuvel wrote:
> On 28 October 2016 at 23:07, Laura Abbott <labbott@...hat.com> wrote:
>>>>> diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
>>>>> new file mode 100644
>>>>> index 0000000..6c271e2
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/mm/physaddr.c
>>>>> @@ -0,0 +1,17 @@
>>>>> +#include <linux/mm.h>
>>>>> +
>>>>> +#include <asm/memory.h>
>>>>> +
>>>>> +unsigned long __virt_to_phys(unsigned long x)
>>>>> +{
>>>>> +    phys_addr_t __x = (phys_addr_t)x;
>>>>> +
>>>>> +    if (__x & BIT(VA_BITS - 1)) {
>>>>> +        /* The bit check ensures this is the right range */
>>>>> +        return (__x & ~PAGE_OFFSET) + PHYS_OFFSET;
>>>>> +    } else {
>>>>> +        VIRTUAL_BUG_ON(x < kimage_vaddr || x > (unsigned long)_end);
>>>>
>>>>
>>>> IIUC, in (3) you were asking if the last check should be '>' or '>='?
>>>>
>>>> To match high_memory, I suspect the latter, as _end doesn't fall within
>>>> the mapped virtual address space.
>>>>
>>>
>>> I was actually concerned about if _end would be correct with KASLR.
>>> Ard confirmed that it gets fixed up to be correct. I'll change the
>>> check to check for >=.
>>>
>>
>> While testing this, I found two places with __pa(_end) to get bounds,
>> one in arm64 code and one in memblock code. x86 gets away with this
>> because memblock is actually __pa_symbol and x86 does image placement
>> different and can check against the maximum image size. I think
>> including _end in __pa_symbol but excluding it from the generic
>> __virt_to_phys makes sense. It's a bit nicer than doing _end - 1 +
>> 1 everywhere.
>>
>
> Could we redefine __pa_symbol() under CONFIG_DEBUG_VIRTUAL to
> something that checks (x >= kimage_vaddr + TEXT_OFFSET || x <=
> (unsigned long)_end), i.e., reject linear virtual addresses? (Assuming
> my understanding of the meaning of __pa_symbol() is correct)
>

Yes, that's going to be my approach. I originally prototyped this
with just x >= kimage_vaddr but kimage_vaddr + TEXT_OFFSET is
a tighter bound.

Thanks,
Laura