From: Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx>
Subject: Re: [PATCH v2 02/20] vmcore: rearrange program headers without assuming consequtive PT_NOTE entries
Date: Tue, 5 Mar 2013 16:36:53 +0800
> ? 2013?03?02? 16:35, HATAYAMA Daisuke ??:
>> Current code assumes all PT_NOTE headers are placed at the beginning
>> of program header table and they are consequtive. But the assumption
>> could be broken by future changes on either kexec-tools or the 1st
>> kernel. This patch removes the assumption and rearranges program
>> headers as the following conditions are satisfied:
>>
>> - PT_NOTE entry is unique at the first entry,
>>
>> - the order of program headers are unchanged during this
>> rearrangement, only their positions are changed in positive
>> direction.
>>
>> - unused part that occurs in the bottom of program headers are filled
>> with 0.
>>
>> Also, this patch adds one exceptional case where the number of PT_NOTE
>> entries is somehow 0. Then, immediately go out of the function.
>>
>> Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
>> ---
>>
>> fs/proc/vmcore.c | 92 +++++++++++++++++++++++++++++++++++++++++++-----------
>> 1 files changed, 74 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
>> index abf4f01..b5c9e33 100644
>> --- a/fs/proc/vmcore.c
>> +++ b/fs/proc/vmcore.c
>> @@ -251,8 +251,7 @@ static u64 __init get_vmcore_size_elf32(char *elfptr)
>> static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz,
>> struct list_head *vc_list)
>> {
>> - int i, nr_ptnote=0, rc=0;
>> - char *tmp;
>> + int i, j, nr_ptnote=0, i_ptnote, rc=0;
>
> After applying the patch, there are two "j" defined.
>
> 251 static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz,
> 252 struct list_head *vc_list)
> 253 {
> 254 int i, j, nr_ptnote=0, i_ptnote, rc=0;
> 255 Elf64_Ehdr *ehdr_ptr;
> 256 Elf64_Phdr phdr, *phdr_ptr;
> 257 Elf64_Nhdr *nhdr_ptr;
> 258 u64 phdr_sz = 0, note_off;
> 259
> 260 ehdr_ptr = (Elf64_Ehdr *)elfptr;
> 261 phdr_ptr = (Elf64_Phdr*)(elfptr + ehdr_ptr->e_phoff);
> 262 for (i = 0; i < ehdr_ptr->e_phnum; i++, phdr_ptr++) {
> 263 int j;
> 264 void *notes_section;
> 265 struct vmcore *new;
>
>
> line 254 and 263.
>
I've already noticed the name of the inner j is never best in meaning
under development but I didn't make patch for it; it's beyond the
scope of this patch series.
I'll replace the outer j by another incremental variable like k.
>
>> Elf64_Ehdr *ehdr_ptr;
>> Elf64_Phdr phdr, *phdr_ptr;
>> Elf64_Nhdr *nhdr_ptr;
>> @@ -302,6 +301,39 @@ static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz,
>> kfree(notes_section);
>> }
>>
>> + if (nr_ptnote == 0)
>> + goto out;
>> +
>> + phdr_ptr = (Elf64_Phdr *)(elfptr + ehdr_ptr->e_phoff);
>> +
>> + /* Remove unwanted PT_NOTE program headers. */
>> +
>> + /* - 1st pass shifts non-PT_NOTE entries until the first
>> + PT_NOTE entry. */
>> + i_ptnote = -1;
>> + for (i = 0; i < ehdr_ptr->e_phnum; ++i) {
>> + if (phdr_ptr[i].p_type == PT_NOTE) {
>> + i_ptnote = i;
>> + break;
>> + }
>> + }
>> + BUG_ON(i_ptnote == -1); /* impossible case since nr_ptnote > 0. */
>> + memmove(phdr_ptr + 1, phdr_ptr, i_ptnote * sizeof(Elf64_Phdr));
>
> is there any problem with this move? What is the batch bytes for every loop
> of memmove?
>
> if i_ptnode == 2, so we have
>
> -------------------------------------
> | PT_LOAD 1 | PT_LOAD 2 | PT_NOTE 1 |
> -------------------------------------
>
> -->
>
> -------------------------------------
> | | PT_LOAD 1 | PT_LOAD 2 |
> -------------------------------------
>
> right? In the move, Does PT_LOAD 1 overwrite the original PT_LOAD 2?
>
Right and yes, see man memmove and man memcpy, and please compare
them.
Thanks.
HATAYAMA, Daisuke
