Re: [PATCH 1/2] arm64: Get 'info->page_offset' from PT_LOAD segments to support KASLR boot cases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kazu,

On Tue, Jul 24, 2018 at 10:47 AM, Bhupesh Sharma <bhsharma@xxxxxxxxxx> wrote:
> Hi Kazu,
>
> On Mon, Jul 23, 2018 at 10:15 PM, Kazuhito Hagio <k-hagio@xxxxxxxxxxxxx> wrote:
>> Hi Bhupesh,
>>
>> On 7/22/2018 3:44 AM, Bhupesh Sharma wrote:
>>> Hello Kazu,
>>>
>>> Many thanks for your review comments.
>>>
>>> On Sat, Jul 21, 2018 at 3:18 AM, Kazuhito Hagio <k-hagio@xxxxxxxxxxxxx> wrote:
>>>>
>>>> On 7/19/2018 1:43 AM, Bhupesh Sharma wrote:
>>>>> The existing methodology to obtain 'info->page_offset' from reading
>>>>> _stext symbol (from kallsyms) doesn't work well in KASLR boot cases on
>>>>> arm64 machines as the PAGE_OFFSET (or the virtual address which
>>>>> indicates the start of the linear region) can be randomized as well
>>>>> on basis of the kaslr-seed.
>>>>>
>>>>> Since the value of PAGE_OFFSET inside the kernel is randomized in such
>>>>> cases and there is no existing mechanism of conveying this value from
>>>>> kernel-space to user-space, so we can use the method used by archs like
>>>>> x86_64 to generate the 'info->page_offset' value from the PT_LOAD
>>>>> segments by subtracting the phy_addr from virt_addr of a PT_LOAD
>>>>> segment.
>>>>>
>>>>> This approach works fine both with KASLR and non-KASLR boot cases.
>>>>>
>>>>> I tested this on my qualcomm-amberwing board. Here are some logs from
>>>>> the KASLR boot cases:
>>>>>
>>>>> - Verify that the EFI firmware supports 'kaslr-seed':
>>>>>
>>>>>       chosen {
>>>>>               kaslr-seed = <0x0 0x0>;
>>>>>               <..snip..>
>>>>>       };
>>>>>
>>>>> - Verify that '--mem-usage' works well after this fix as well (I used
>>>>>   kernel 4.18.0-rc4+ for my checks):
>>>>>
>>>>> The kernel version is not supported.
>>>>> The makedumpfile operation may be incomplete.
>>>>>
>>>>> TYPE          PAGES                   EXCLUDABLE      DESCRIPTION
>>>>> ----------------------------------------------------------------------
>>>>> ZERO          4396                    yes             Pages filled
>>>>> with zero
>>>>> NON_PRI_CACHE 27859                   yes             Cache pages
>>>>> without private flag
>>>>> PRI_CACHE     18490                   yes             Cache pages with
>>>>> private flag
>>>>> USER          2728                    yes             User process
>>>>> pages
>>>>> FREE          1465848                 yes             Free pages
>>>>> KERN_DATA     18537                   no              Dumpable kernel
>>>>> data
>>>>>
>>>>> page size:            65536
>>>>> Total pages on system:        1537858
>>>>> Total size on system: 100785061888     Byte
>>>>>
>>>>> Signed-off-by: Bhupesh Sharma <bhsharma@xxxxxxxxxx>
>>>>> ---
>>>>>  arch/arm64.c   | 23 ++++++++++++++++++-----
>>>>>  common.h       |  1 +
>>>>>  makedumpfile.h |  1 +
>>>>>  3 files changed, 20 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64.c b/arch/arm64.c
>>>>> index 2fd3e1874376..9e8c77c76935 100644
>>>>> --- a/arch/arm64.c
>>>>> +++ b/arch/arm64.c
>>>>> @@ -265,6 +265,9 @@ get_xen_info_arm64(void)
>>>>>  int
>>>>>  get_versiondep_info_arm64(void)
>>>>>  {
>>>>> +     int i;
>>>>> +     unsigned long long phys_start;
>>>>> +     unsigned long long virt_start;
>>>>>       ulong _stext;
>>>>>
>>>>>       _stext = get_stext_symbol();
>>>>> @@ -289,12 +292,22 @@ get_versiondep_info_arm64(void)
>>>>>               return FALSE;
>>>>>       }
>>>>>
>>>>
>>>>> -     info->page_offset = (0xffffffffffffffffUL) << (va_bits - 1);
>>>>> -
>>>>> -     DEBUG_MSG("page_offset=%lx, va_bits=%d\n", info->page_offset,
>>>>> -                     va_bits);
>>>>
>>>> According to makedumpfile commit 4944f93484 ("x86_64: Calculate page_offset
>>>> in case of re-filtering/sadump/virsh dump"), in case of re-filtering,
>>>> we don't have any PT_LOAD.  So, with this patch, we cannot re-filter
>>>> dump file on arm64 system?
>>>>
>>>> If we cannot, is it better to leave the above behind the following
>>>> pt_load section for re-filtering non-KASLR dump?
>>>>
>>>>> +     if (get_num_pt_loads()) {
>>>>> +             for (i = 0;
>>>>> +                     get_pt_load(i, &phys_start, NULL, &virt_start, NULL);
>>>>> +                     i++) {
>>>>> +                     if (virt_start != NOT_KV_ADDR
>>>>> +                                     && virt_start < __START_KERNEL_map
>>>>> +                                     && phys_start != NOT_PADDR && phys_start != NOT_PADDR_ARM64) {
>>>>> +                             info->page_offset = virt_start - phys_start;
>>>>> +                             DEBUG_MSG("info->page_offset: %lx, VA_BITS: %d\n",
>>>>> +                                             info->page_offset, va_bits);
>>>>> +                             return TRUE;
>>>>> +                     }
>>>>> +             }
>>>>> +     }
>>>>
>>>> I'll adjust some indents for readability.
>>>>
>>>>>
>>>>> -     return TRUE;
>>>>> +     return FALSE;
>>>>>  }
>>>>>
>>>>>  /*
>>>>> diff --git a/common.h b/common.h
>>>>> index 6e2f657a79c7..a8181777dbb7 100644
>>>>> --- a/common.h
>>>>> +++ b/common.h
>>>>> @@ -48,6 +48,7 @@
>>>>>  #define NOT_MEMMAP_ADDR      (0x0)
>>>>>  #define NOT_KV_ADDR  (0x0)
>>>>>  #define NOT_PADDR    (ULONGLONG_MAX)
>>>>
>>>>> +#define NOT_PADDR_ARM64      (0x0000000010a80000UL)
>>>>
>>>> I think that this should not be in common.h, because it's not for
>>>> arch-specific definitions, so I'll move it to makedumpfile.h.
>>>>
>>>>>  #define BADADDR      ((ulong)(-1))
>>>>>
>>>>>  #endif  /* COMMON_H */
>>>>> diff --git a/makedumpfile.h b/makedumpfile.h
>>>>> index 5ff94b8e4ac6..5297279f0f3b 100644
>>>>> --- a/makedumpfile.h
>>>>> +++ b/makedumpfile.h
>>>>> @@ -2020,6 +2020,7 @@ struct domain_list {
>>>>>  #define MFNS_PER_FRAME               (info->page_size / sizeof(unsigned long))
>>>>>
>>>>>  #ifdef __aarch64__
>>>>> +#define __START_KERNEL_map           (0xffffffff80000000UL)
>>>>
>>>> This #ifdef here is for the definitions related to Xen extraction.
>>>> I'll move it to the same place as the above.
>>>>
>>>>>  unsigned long long kvtop_xen_arm64(unsigned long kvaddr);
>>>>>  #define kvtop_xen(X) kvtop_xen_arm64(X)
>>>>>  #endif /* aarch64 */
>>>>>
>>>>
>>>> I attached a patch that was applied my suggestions.
>>>> How about it?
>>>
>>> I agree. The patch looks good with the suggested changes.
>>> Please go ahead and feel free to apply the modified patch.
>>
>> OK, I'll apply it.
>>
>> With respect to the 2/2 patch, I'm waiting for the kernel patch
>> to be merged.
>>
>> If need be, I can apply the 1/2 patch separately from the 2/2 patch,
>> because I think they are not directly connected with each other.
>
> Sure, I think we can handle 2/2 patch when it's kernel part is
> reviewed properly by the arm64 kernel maintainer(s). We can apply 1/2
> patch in the meanwhile.
>
>> p.s. it seems that I cannot send emails to gmail for now,
>> so dropped your gmail address. sorry for the inconvenience.
>
> Sure, I use the gmail address just as a backup to read/answer upstream
> replies once I am out of office (as it is easier to access on my
> phone).
>
> Thanks,
> Bhupesh

The kernel patch is now accepted in Linus's tree:
commit e401b7c2c69008ad2fcdc154f7c5421281c90042
Author: Bhupesh Sharma <bhsharma@xxxxxxxxxx>
Date:   Mon Jul 30 11:54:43 2018 +0530

    arm64, kaslr: export offset in VMCOREINFO ELF notes

Can you please pick this patchset and now apply it to makedumpfile source?

Thanks,
Bhupesh

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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux