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;
...
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
