linux-kernel - Re: [PATCH] /proc/kcore: Update physical address for kcore ram and text

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:   Tue, 14 Feb 2017 07:16:22 +0530
From:   Pratyush Anand <panand@...hat.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     open list <linux-kernel@...r.kernel.org>,
        Baoquan He <bhe@...hat.com>, Dave Young <dyoung@...hat.com>,
        Dave Anderson <anderson@...hat.com>,
        Kexec Mailing List <kexec@...ts.infradead.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] /proc/kcore: Update physical address for kcore ram and
 text



On Tuesday 14 February 2017 03:55 AM, Kees Cook wrote:
> On Mon, Jan 30, 2017 at 11:00 AM, Pratyush Anand <panand@...hat.com> wrote:
>> CCing Andrew and Kees for their review comments.
>>
>>
>> On Wednesday 25 January 2017 10:14 AM, Pratyush Anand wrote:
>>> Currently all the p_paddr of PT_LOAD headers are assigned to 0, which is
>>> not true and could be misleading, since 0 is a valid physical address.
>>>
>>> User space tools like makedumpfile needs to know physical address for
>>> PT_LOAD segments of direct mapped regions. Therefore this patch updates
>>> paddr for such regions. It also sets an invalid paddr (-1) for other
>>> regions, so that user space tool can know whether a physical address
>>> provided in PT_LOAD is correct or not.
>>>
>>> Signed-off-by: Pratyush Anand <panand@...hat.com>
>>> ---
>>> fs/proc/kcore.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
>>> index 0b80ad87b4d6..ea9f3d1ae830 100644
>>> --- a/fs/proc/kcore.c
>>> +++ b/fs/proc/kcore.c
>>> @@ -373,7 +373,10 @@ static void elf_kcore_store_hdr(char *bufp, int
>>> nphdr, int dataoff)
>>> phdr->p_flags = PF_R|PF_W|PF_X;
>>> phdr->p_offset = kc_vaddr_to_offset(m->addr) + dataoff;
>>> phdr->p_vaddr = (size_t)m->addr;
>>> - phdr->p_paddr = 0;
>>> + if (m->type == KCORE_RAM || m->type == KCORE_TEXT)
>>> + phdr->p_paddr = __pa(m->addr);
>>> + else
>>> + phdr->p_paddr = (elf_addr_t)-1;
>>> phdr->p_filesz = phdr->p_memsz = m->size;
>>> phdr->p_align = PAGE_SIZE;
>>> }
>>>
>
> Well, CONFIG_PROC_KCORE is a generalized root KASLR exposure (though
> there are lots of such exposures). Why is the actual physical address
> needed? Can this just report the virtual address instead? Then the
> tool can build a map, but it looks like an identity map, rather than
> creating a new physical/virtual memory ASLR offset exposure?

Well, having an ASLR offset information can help to translate an 
identity mapped virtual address to a physical address. But that would be 
an additional field in PT_LOAD header structure and an arch dependent value.

Moreover, sending a valid physical address like 0 does not seem right. 
So, IMHO it is better to fix that and send valid physical address when 
available (identity mapped).

Thanks for the review.

~Pratyush