Hi,

On 12.12.2019 17:12, Kazuhito Hagio wrote:
> Hi Mikhail,
> 
>> -----Original Message-----
>> Hello Kazu,
>>
>> I think we can try to generalize the kaslr offset extraction.
>> I won't speak for other architectures, but for s390 that 
>> get_kaslr_offset_arm64()
>> should work fine. The only concern of mine is this TODO statement:
>>
>> if (_text <= vaddr && vaddr <= _end) {
>>      DEBUG_MSG("info->kaslr_offset: %lx\n", info->kaslr_offset);
>>      return info->kaslr_offset;
>>      } else {
>>      /*
>>      * TODO: we need to check if it is vmalloc/vmmemmap/module
>>      * address, we will have different offset
>>      */
>>      return 0;
>> }
>>
>> Could you explain this one?
> 
> Probably it was considered that the check would be needed to support
> the whole KASLR behavior when get_kaslr_offset_x86_64() was written
> originally.
> 
> But in the current makedumpfile for x86_64 and arm64 supporting KASLR,
> the offset we need is the one for symbol addresses in vmlinux only.
> As I said below, module symbol addresses are retrieved from vmcore.
> Other addresses should not be passed to the function for now, as far
> as I know.
> 
> So I think the TODO comment is confusing, and it would be better to
> remove it or change it to something like:
> /*
>  * Returns 0 if vaddr does not need the offset to be added,
>  * e.g. for module address.
>  */
> 
> But if s390 uses get_kaslr_offset() in its arch-specific code to
> adjust addresses other than kernel text address, we might need to
> modify it for s390, not generalize it.

Currently, s390 doesn't use get_kaslr_offset() in its arch-specific 
code. 

> 
> Thanks,
> Kazu
> 
>>
>> Thanks,
>> Mikhail
>>
>> On 09.12.2019 23:02, Kazuhito Hagio wrote:
>>> Hi Mikhail,
>>>
>>> Sorry for late reply.
>>>
>>>> -----Original Message-----
>>>> Since kernel v5.2 KASLR is supported on s390. In makedumpfile however no
>>>> support has been added yet. This patch adds the arch specific function
>>>> get_kaslr_offset() for s390x.
>>>> Since the values in vmcoreinfo are already relocated, the patch is
>>>> mainly relevant for vmlinux processing (-x option).
>>>
>>> In the current implementation of makedumpfile, the get_kaslr_offset(vaddr)
>>> is supposed to return the KASLR offset only when the offset is needed to
>>> add to the vaddr.  So generally symbols from kernel (vmlinux) need it, but
>>> symbols from modules are resolved dynamically and don't need the offset.
>> \>
>>> This patch always returns the offset if any, as a result, I guess this patch
>>> will not work as expected with module symbols in filter config file.
>>>
>>> So... How about making get_kaslr_offset_arm64() general for other archs
>>> (get_kaslr_offset_general() or something), then using it also for s390?
>>> If OK, I can do that generalization.
>>>
>>> Thanks,
>>> Kazu
>>>
>>>>
>>>> Signed-off-by: Philipp Rudo <pr...@linux.ibm.com>
>>>> Signed-off-by: Mikhail Zaslonko <zaslo...@linux.ibm.com>
>>>> ---
>>>>  arch/s390x.c   | 32 ++++++++++++++++++++++++++++++++
>>>>  makedumpfile.h |  3 ++-
>>>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/s390x.c b/arch/s390x.c
>>>> index bf9d58e..892df14 100644
>>>> --- a/arch/s390x.c
>>>> +++ b/arch/s390x.c
>>>> @@ -122,6 +122,38 @@ get_machdep_info_s390x(void)
>>>>    return TRUE;
>>>>  }
>>>>
>>>> +unsigned long
>>>> +get_kaslr_offset_s390x(unsigned long vaddr)
>>>> +{
>>>> +  unsigned int i;
>>>> +  char buf[BUFSIZE_FGETS], *endp;
>>>> +
>>>> +  if (!info->file_vmcoreinfo)
>>>> +          return FALSE;
>>>> +
>>>> +  if (fseek(info->file_vmcoreinfo, 0, SEEK_SET) < 0) {
>>>> +          ERRMSG("Can't seek the vmcoreinfo file(%s). %s\n",
>>>> +                 info->name_vmcoreinfo, strerror(errno));
>>>> +          return FALSE;
>>>> +  }
>>>> +
>>>> +  while (fgets(buf, BUFSIZE_FGETS, info->file_vmcoreinfo)) {
>>>> +          i = strlen(buf);
>>>> +          if (!i)
>>>> +                  break;
>>>> +          if (buf[i - 1] == '\n')
>>>> +                  buf[i - 1] = '\0';
>>>> +          if (strncmp(buf, STR_KERNELOFFSET,
>>>> +                      strlen(STR_KERNELOFFSET)) == 0) {
>>>> +                  info->kaslr_offset =
>>>> +                          strtoul(buf + strlen(STR_KERNELOFFSET), &endp, 
>>>> 16);
>>>> +                  DEBUG_MSG("info->kaslr_offset: %lx\n", 
>>>> info->kaslr_offset);
>>>> +          }
>>>> +  }
>>>> +
>>>> +  return info->kaslr_offset;
>>>> +}
>>>> +
>>>>  static int
>>>>  is_vmalloc_addr_s390x(unsigned long vaddr)
>>>>  {
>>>> diff --git a/makedumpfile.h b/makedumpfile.h
>>>> index ac11e90..26f6247 100644
>>>> --- a/makedumpfile.h
>>>> +++ b/makedumpfile.h
>>>> @@ -1071,11 +1071,12 @@ unsigned long long vaddr_to_paddr_ppc(unsigned 
>>>> long vaddr);
>>>>  int get_machdep_info_s390x(void);
>>>>  unsigned long long vaddr_to_paddr_s390x(unsigned long vaddr);
>>>>  int is_iomem_phys_addr_s390x(unsigned long addr);
>>>> +unsigned long get_kaslr_offset_s390x(unsigned long vaddr);
>>>>  #define find_vmemmap()            stub_false()
>>>>  #define get_phys_base()           stub_true()
>>>>  #define get_machdep_info()        get_machdep_info_s390x()
>>>>  #define get_versiondep_info()     stub_true()
>>>> -#define get_kaslr_offset(X)       stub_false()
>>>> +#define get_kaslr_offset(X)       get_kaslr_offset_s390x(X)
>>>>  #define vaddr_to_paddr(X) vaddr_to_paddr_s390x(X)
>>>>  #define paddr_to_vaddr(X) paddr_to_vaddr_general(X)
>>>>  #define is_phys_addr(X)           is_iomem_phys_addr_s390x(X)
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
> 
> 

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

Reply via email to