Message-ID: <9debc20e-1460-4400-a9ca-50b407948976@oracle.com>
Date: Thu, 27 Feb 2025 22:47:04 -0800
From: Indu Bhagat <indu.bhagat@...cle.com>
To: Puranjay Mohan <puranjay@...nel.org>, Weinan Liu <wnliu@...gle.com>
Cc: irogers@...gle.com, joe.lawrence@...hat.com, jpoimboe@...nel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-toolchains@...r.kernel.org, live-patching@...r.kernel.org,
        mark.rutland@....com, peterz@...radead.org, roman.gushchin@...ux.dev,
        rostedt@...dmis.org, will@...nel.org
Subject: Re: [PATCH 0/8] unwind, arm64: add sframe unwinder for kernel

On 2/27/25 1:38 AM, Puranjay Mohan wrote:
> Indu Bhagat <indu.bhagat@...cle.com> writes:
> 
>> On 2/26/25 2:23 AM, Puranjay Mohan wrote:
>>> Indu Bhagat <indu.bhagat@...cle.com> writes:
>>>
>>>> On 2/25/25 3:54 PM, Weinan Liu wrote:
>>>>> On Tue, Feb 25, 2025 at 11:38 AM Indu Bhagat <indu.bhagat@...cle.com> wrote:
>>>>>>
>>>>>> On Mon, Feb 10, 2025 at 12:30 AM Weinan Liu <wnliu@...gle.com> wrote:
>>>>>>>> I already have a WIP patch to add sframe support to the kernel module.
>>>>>>>> However, it is not yet working. I had trouble unwinding frames for the
>>>>>>>> kernel module using the current algorithm.
>>>>>>>>
>>>>>>>> Indu has likely identified the issue and will be addressing it from the
>>>>>>>> toolchain side.
>>>>>>>>
>>>>>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=32666
>>>>>>>
>>>>>>> I have a working in progress patch that adds sframe support for kernel
>>>>>>> module.
>>>>>>> https://github.com/heuza/linux/tree/sframe_unwinder.rfc
>>>>>>>
>>>>>>> According to the sframe table values I got during runtime testing, looks
>>>>>>> like the offsets are not correct .
>>>>>>>
>>>>>>
>>>>>> I hope to sanitize the fix for 32666 and post upstream soon (I had to
>>>>>> address other related issues).  Unless fixed, relocating .sframe
>>>>>> sections using the .rela.sframe is expected to generate incorrect output.
>>>>>>
>>>>>>> When unwind symbols init_module(0xffff80007b155048) from the kernel
>>>>>>> module(livepatch-sample.ko), the start_address of the FDE entries in the
>>>>>>> sframe table of the kernel modules appear incorrect.
>>>>>>
>>>>>> init_module will apply the relocations on the .sframe section, isnt it ?
>>>>>>
>>>>>>> For instance, the first FDE's start_addr is reported as -20564. Adding
>>>>>>> this offset to the module's sframe section address (0xffff80007b15a040)
>>>>>>> yields 0xffff80007b154fec, which is not within the livepatch-sample.ko
>>>>>>> memory region(It should be larger than 0xffff80007b155000).
>>>>>>>
>>>>>>
>>>>>> Hmm..something seems off here.  Having tested a potential fix for 32666
>>>>>> locally, I do not expect the first FDE to show this symptom.
>>>>>>
>>>>>
>>>
>>> Hi,
>>>
>>> Sorry for not responding in the past few days.  I was on PTO and was
>>> trying to improve my snowboarding technique, I am back now!!
>>>
>>> I think what we are seeing is expected behaviour:
>>>
>>>    | For instance, the first FDE's start_addr is reported as -20564. Adding
>>>    | this offset to the module's sframe section address (0xffff80007b15a040)
>>>    | yields 0xffff80007b154fec, which is not within the livepatch-sample.ko
>>>    | memory region(It should be larger than 0xffff80007b155000).
>>>
>>>
>>> Let me explain using a __dummy__ example.
>>>
>>> Assume Memory layout before relocation:
>>>
>>>    | Address | Element                                 | Relocation
>>>    |  ....   | ....                                    |
>>>    |   60    | init_module (start address)             |
>>>    |   72    | init_module (end address)               |
>>>    |  ....   | .....                                   |
>>>    |   100   | Sframe section header start address     |
>>>    |   128   | First FDE's start address               | RELOC_OP_PREL -> Put init_module address (60) - current address (128)
>>>
>>> So, after relocation First FDE's start address has value 60 - 128 = -68
>>>
>>
>> For SFrame FDE function start address is :
>>
>> "Signed 32-bit integral field denoting the virtual memory address of the
>> described function, for which the SFrame FDE applies.  The value encoded
>> in the ‘sfde_func_start_address’ field is the offset in bytes of the
>> function’s start address, from the SFrame section."
>>
>> So, in your case, after applying the relocations, you will get:
>> S + A - P = 60 - 128 = -68
>>
>> This is the distance of the function start address (60) from the current
>> location in SFrame section (128)
>>
>> But what we intend to store is the distance of the function start
>> address from the start of the SFrame section.  So we need to do an
>> additional step for SFrame FDE:  Value += r_offset
> 
> Thanks for the explaination, now it makes sense.
> 
> But I couldn't find a relocation type in AARCH64 that does this extra +=
> r_offset along with PREL32.
> 
> The kernel's module loader is only doing the R_AARCH64_PREL32 which is
> why we see this issue.
> 
> How is this working even for the kernel itself? or for that matter, any
> other binary compiled with sframe?
> 

For the usual executables or shared objects, the calculations are 
applied by ld.bfd at this time.  Hence, the issue manifests in 
relocatable files.

>  From my limited undestanding, the way to fix this would be to hack the
> relocator to do this additional step while relocating .sframe sections.
> Or the 'addend' values in .rela.sframe should already have the +r_offset
> added to it, then no change to the relocator would be needed.
> 

Of the two, adjusting the addend values in .rela.sframe may be a 
reasonable way to go about it.  Let me try it out in GAS and ld.bfd.

>> -68 + 28 = -40
>> Where 28 is the r_offset in the RELA.
>>
>> So we really expect a -40 in the relocated SFrame section instead of -68
>> above.  IOW, the RELAs of SFrame section will need an additional step
>> after relocation.
>>
> 
> Thanks,
> Puranjay

Powered by blists - more mailing lists