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 <prudo@xxxxxxxxxxxxx> >>>> Signed-off-by: Mikhail Zaslonko <zaslonko@xxxxxxxxxxxxx> >>>> --- >>>> 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@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec