[PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()

[Pratyush Anand]

On Thu, Oct 6, 2016 at 6:58 PM, Matthias Brugger <matthias.bgg at gmail.com> wrote:
> On 09/07/2016 06:33 AM, AKASHI Takahiro wrote:
>>
>> From: Pratyush Anand <panand at redhat.com>
>>
>> get_kernel_stext_sym() has been defined for both arm and i386. Other
>> architecture might need some other kernel symbol address. Therefore
>> rewrite
>> this function as generic function to get any kernel symbol address.
>>
>> More over, kallsyms is not arch specific representation, therefore have
>> common function for all arches.
>>
>> Signed-off-by: Pratyush Anand <panand at redhat.com>
>> [created symbols.c]
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> ---
>>  kexec/Makefile                  |  1 +
>>  kexec/arch/arm/crashdump-arm.c  | 40
>> +---------------------------------------
>>  kexec/arch/i386/crashdump-x86.c | 32 +-------------------------------
>>  kexec/kexec.h                   |  2 ++
>>  kexec/symbols.c                 | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 46 insertions(+), 70 deletions(-)
>>  create mode 100644 kexec/symbols.c
>>
>> diff --git a/kexec/Makefile b/kexec/Makefile
>> index 39f365f..2b4fb3d 100644
>> --- a/kexec/Makefile
>> +++ b/kexec/Makefile
>> @@ -26,6 +26,7 @@ KEXEC_SRCS_base += kexec/kernel_version.c
>>  KEXEC_SRCS_base += kexec/lzma.c
>>  KEXEC_SRCS_base += kexec/zlib.c
>>  KEXEC_SRCS_base += kexec/kexec-xen.c
>> +KEXEC_SRCS_base += kexec/symbols.c
>>
>>  KEXEC_GENERATED_SRCS += $(PURGATORY_HEX_C)
>>
>> diff --git a/kexec/arch/arm/crashdump-arm.c
>> b/kexec/arch/arm/crashdump-arm.c
>> index 4a89b5e..2bc898b 100644
>> --- a/kexec/arch/arm/crashdump-arm.c
>> +++ b/kexec/arch/arm/crashdump-arm.c
>> @@ -73,48 +73,10 @@ static struct crash_elf_info elf_info = {
>>
>>  extern unsigned long long user_page_offset;
>>
>> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
>> -static unsigned long long get_kernel_stext_sym(void)
>> -{
>> -       const char *kallsyms = "/proc/kallsyms";
>> -       const char *stext = "_stext";
>> -       char sym[128];
>> -       char line[128];
>> -       FILE *fp;
>> -       unsigned long long vaddr = 0;
>> -       char type;
>> -
>> -       fp = fopen(kallsyms, "r");
>> -       if (!fp) {
>> -               fprintf(stderr, "Cannot open %s\n", kallsyms);
>> -               return 0;
>> -       }
>> -
>> -       while(fgets(line, sizeof(line), fp) != NULL) {
>> -               unsigned long long addr;
>> -
>> -               if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
>> -                       continue;
>> -
>> -               if (strcmp(sym, stext) == 0) {
>> -                       dbgprintf("kernel symbol %s vaddr = %#llx\n",
>> stext, addr);
>> -                       vaddr = addr;
>> -                       break;
>> -               }
>> -       }
>> -
>> -       fclose(fp);
>> -
>> -       if (vaddr == 0)
>> -               fprintf(stderr, "Cannot get kernel %s symbol address\n",
>> stext);
>> -
>> -       return vaddr;
>> -}
>> -
>>  static int get_kernel_page_offset(struct kexec_info *info,
>>                 struct crash_elf_info *elf_info)
>>  {
>> -       unsigned long long stext_sym_addr = get_kernel_stext_sym();
>> +       unsigned long long stext_sym_addr = get_kernel_sym("stext");
>>         if (stext_sym_addr == 0) {
>>                 if (user_page_offset != (-1ULL)) {
>>                         elf_info->page_offset = user_page_offset;
>> diff --git a/kexec/arch/i386/crashdump-x86.c
>> b/kexec/arch/i386/crashdump-x86.c
>> index bbc0f35..664eb3b 100644
>> --- a/kexec/arch/i386/crashdump-x86.c
>> +++ b/kexec/arch/i386/crashdump-x86.c
>> @@ -102,36 +102,6 @@ static int get_kernel_paddr(struct kexec_info
>> *UNUSED(info),
>>         return -1;
>>  }
>>
>> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
>> -static unsigned long long get_kernel_stext_sym(void)
>> -{
>> -       const char *kallsyms = "/proc/kallsyms";
>> -       const char *stext = "_stext";
>> -       char sym[128];
>> -       char line[128];
>> -       FILE *fp;
>> -       unsigned long long vaddr;
>> -       char type;
>> -
>> -       fp = fopen(kallsyms, "r");
>> -       if (!fp) {
>> -               fprintf(stderr, "Cannot open %s\n", kallsyms);
>> -               return 0;
>> -       }
>> -
>> -       while(fgets(line, sizeof(line), fp) != NULL) {
>> -               if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
>> -                       continue;
>> -               if (strcmp(sym, stext) == 0) {
>> -                       dbgprintf("kernel symbol %s vaddr = %16llx\n",
>> stext, vaddr);
>> -                       return vaddr;
>> -               }
>> -       }
>> -
>> -       fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
>> -       return 0;
>> -}
>> -
>>  /* Retrieve info regarding virtual address kernel has been compiled for
>> and
>>   * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
>>   * from kexec-tools fails because of malformed elf notes. A kernel patch
>> has
>> @@ -182,7 +152,7 @@ static int get_kernel_vaddr_and_size(struct kexec_info
>> *UNUSED(info),
>>
>>         /* Traverse through the Elf headers and find the region where
>>          * _stext symbol is located in. That's where kernel is mapped */
>> -       stext_sym = get_kernel_stext_sym();
>> +       stext_sym = get_kernel_sym("stext");
>
>
>
> I think this should be get_kernel_sym("_stext");

OK.

>
> Apart from that as Simon already mentioned, due to commit
> 9f62cbd ("kexec/arch/i386: Add support for KASLR memory randomization")
> this patch does not apply cleanly.
>

Will rebase with latest.

Thanks for your review.

~Pratyush