linux-kernel - Re: [PATCH 02/13] vmcore: round up buffer size of ELF headers by PAGE

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: <20130215151821.GB27784@redhat.com>
Date:	Fri, 15 Feb 2013 10:18:21 -0500
From:	Vivek Goyal <vgoyal@...hat.com>
To:	HATAYAMA Daisuke <d.hatayama@...fujitsu.com>
Cc:	ebiederm@...ssion.com, cpw@....com,
	kumagai-atsushi@....nes.nec.co.jp, lisa.mitchell@...com,
	kexec@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/13] vmcore: round up buffer size of ELF headers by
 PAGE_SIZE

On Thu, Feb 14, 2013 at 07:11:54PM +0900, HATAYAMA Daisuke wrote:
> To satisfy mmap() page-size boundary requirement, reound up buffer
> size of ELF headers by PAGE_SIZE. The resulting value becomes offset
> of ELF note segments and it's assigned in unique PT_NOTE program
> header entry.
> 
> Also, some part that assumes past ELF headers' size is replaced by
> this new rounded-up value.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama@...fujitsu.com>
> ---
> 
>  fs/proc/vmcore.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 85714c3..5010ead 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -313,7 +313,7 @@ static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz,
>  	phdr.p_flags   = 0;
>  	note_off = sizeof(Elf64_Ehdr) +
>  			(ehdr_ptr->e_phnum - nr_ptnote +1) * sizeof(Elf64_Phdr);
> -	phdr.p_offset  = note_off;
> +	phdr.p_offset  = roundup(note_off, PAGE_SIZE);
>  	phdr.p_vaddr   = phdr.p_paddr = 0;
>  	phdr.p_filesz  = phdr.p_memsz = phdr_sz;
>  	phdr.p_align   = 0;
> @@ -331,6 +331,8 @@ static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz,
>  	/* Modify e_phnum to reflect merged headers. */
>  	ehdr_ptr->e_phnum = ehdr_ptr->e_phnum - nr_ptnote + 1;
>  

Hi Hatayama,

While reading the /proc/vmcore code again, I realized that we are making
a horrible assumption. And that is that all PT_NOTE program headers
prepared by kexec-tools are contiguous. And we also seem to be assuming
that all PT_NOTE phdrs are following immediately Elf Header.

        /* Add merged PT_NOTE program header*/
        tmp = elfptr + sizeof(Elf64_Ehdr);
        memcpy(tmp, &phdr, sizeof(phdr));
        tmp += sizeof(phdr);

        /* Remove unwanted PT_NOTE program headers. */
        i = (nr_ptnote - 1) * sizeof(Elf64_Phdr);
        *elfsz = *elfsz - i;
        memmove(tmp, tmp+i, ((*elfsz)-sizeof(Elf64_Ehdr)-sizeof(Elf64_Phdr)));

I know I wrote this code but now I realize that this is very bad
assumption. We should not be assuming where PT_NOTE headers are and
also should not be assuming that these are contiguous.

This will require fixing. I think we just need to read old elf headers
in a buffer and prepare new headers (merged one) in a separate buffer
instead of trying to make do with single buffer.

If it is not too much of trouble, can you please do this cleanup and
rebase your patches on top of it.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/