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

[Pratyush Anand <panand@redhat.com>]

On Tuesday 14 February 2017 03:55 AM, Kees Cook wrote:
> On Mon, Jan 30, 2017 at 11:00 AM, Pratyush Anand <panand@redhat.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@redhat.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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec