在 2019年12月26日 11:38, lijiang 写道:
> Hi, Kazu and Mikhail,
>
>> Hi Mikhail,
>>
>>> -----Original Message-----
>>> 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.
>>
>> OK, I pushed a patch that generalizes it to my test repository.
>> Could you enable s390 to use it and test?
>> https://github.com/k-hagio/makedumpfile/tree/add-get_kaslr_offset_general
>>
>
> I enabled it on s390 as below and tested, it worked.
>
> @@ -1075,7 +1075,7 @@ int is_iomem_phys_addr_s390x(unsigned long addr);
> #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_general(X)
> #define vaddr_to_paddr(X) vaddr_to_paddr_s390x(X)
>
> But, there is still a problem that needs to be improved. In the find_kaslr_offsets(),
> the value of SYMBOL(_stext) is always 0(zero) and it is passed to the get_kaslr_offset().
> For the following code in the get_kaslr_offset_general(), it does not work as expected.
> ...
> if (_text <= vaddr && vaddr <= _end)
> return info->kaslr_offset;
> else
> return 0;
In addition, the above code confused me, it will always return 0 on s390(please refer to my logs).
Thanks.
> ...
> Here is my log:
> get_kaslr_offset_general: info->kaslr_offset: 67ebc000, _text:100000, _end:10ba000, vaddr:0
>
> After applied the following patch, got the expected result.
> int
> find_kaslr_offsets()
> {
> @@ -3973,6 +4042,11 @@ find_kaslr_offsets()
> * called this function between open_vmcoreinfo() and
> * close_vmcoreinfo()
> */
> + READ_SYMBOL("_stext", _stext);
> + if (SYMBOL(_stext) == NOT_FOUND_SYMBOL) {
> + ERRMSG("Can't get the symbol of _stext.\n");
> + goto out;
> + }
> get_kaslr_offset(SYMBOL(_stext));
>
> Here is my log:
> get_kaslr_offset_general: info->kaslr_offset: 67ebc000, _text:100000, _end:10ba000, vaddr:67fbc000
>
> Basically, before using the value of SYMBOL(_stext), need to ensure that the SYMBOL(_stext) is parsed
> correctly.
>
> Thanks.
>
>> Thanks,
>> Kazu
>>
>>>
>>>>
>>>> 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
>>
>
>
> _______________________________________________
> kexec mailing list
> kexec@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/kexec
>
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
