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