linux-kernel - Re: [PATCH 0/8] unwind, arm64: add sframe unwinder for kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <mb61p1pw0qrpi.fsf@kernel.org>
Date: Fri, 14 Feb 2025 18:58:01 +0000
From: Puranjay Mohan <puranjay@...nel.org>
To: Indu Bhagat <indu.bhagat@...cle.com>, Song Liu <song@...nel.org>, Josh
 Poimboeuf <jpoimboe@...nel.org>
Cc: Weinan Liu <wnliu@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
 Peter Zijlstra <peterz@...radead.org>, Mark Rutland
 <mark.rutland@....com>, roman.gushchin@...ux.dev, Will Deacon
 <will@...nel.org>, Ian Rogers <irogers@...gle.com>,
 linux-toolchains@...r.kernel.org, linux-kernel@...r.kernel.org,
 live-patching@...r.kernel.org, joe.lawrence@...hat.com,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/8] unwind, arm64: add sframe unwinder for kernel

Indu Bhagat <indu.bhagat@...cle.com> writes:

> On 2/14/25 9:39 AM, Indu Bhagat wrote:
>> On 2/13/25 11:57 PM, Puranjay Mohan wrote:
>>> Indu Bhagat <indu.bhagat@...cle.com> writes:
>>>
>>>> On 2/12/25 11:25 PM, Song Liu wrote:
>>>>> On Wed, Feb 12, 2025 at 6:45 PM Josh Poimboeuf <jpoimboe@...nel.org> 
>>>>> wrote:
>>>>>>
>>>>>> On Wed, Feb 12, 2025 at 06:36:04PM -0800, Song Liu wrote:
>>>>>>>>> [   81.261748]  copy_process+0xfdc/0xfd58 
>>>>>>>>> [livepatch_special_static]
>>>>>>>>
>>>>>>>> Does that copy_process+0xfdc/0xfd58 resolve to this line in
>>>>>>>> copy_process()?
>>>>>>>>
>>>>>>>>                           refcount_inc(&current->signal->sigcnt);
>>>>>>>>
>>>>>>>> Maybe the klp rela reference to 'current' is bogus, or resolving 
>>>>>>>> to the
>>>>>>>> wrong address somehow?
>>>>>>>
>>>>>>> It resolves the following line.
>>>>>>>
>>>>>>> p->signal->tty = tty_kref_get(current->signal->tty);
>>>>>>>
>>>>>>> I am not quite sure how 'current' should be resolved.
>>>>>>
>>>>>> Hm, on arm64 it looks like the value of 'current' is stored in the
>>>>>> SP_EL0 register.  So I guess that shouldn't need any relocations.
>>>>>>
>>>>>>> The size of copy_process (0xfd58) is wrong. It is only about
>>>>>>> 5.5kB in size. Also, the copy_process function in the .ko file
>>>>>>> looks very broken. I will try a few more things.
>>>>>
>>>>> When I try each step of kpatch-build, the copy_process function
>>>>> looks reasonable (according to gdb-disassemble) in fork.o and
>>>>> output.o. However, copy_process looks weird in livepatch-special- 
>>>>> static.o,
>>>>> which is generated by ld:
>>>>>
>>>>> ld -EL  -maarch64linux -z norelro -z noexecstack
>>>>> --no-warn-rwx-segments -T ././kpatch.lds  -r -o
>>>>> livepatch-special-static.o ./patch-hook.o ./output.o
>>>>>
>>>>> I have attached these files to the email. I am not sure whether
>>>>> the email server will let them through.
>>>>>
>>>>> Indu, does this look like an issue with ld?
>>>>>
>>>>
>>>> Sorry for the delay.
>>>>
>>>> Looks like there has been progress since and issue may be elsewhere, 
>>>> but:
>>>>
>>>> FWIW, I looked at the .sframe and .rela.sframe sections here, the data
>>>> does look OK.  I noted that there is no .sframe for copy_process () in
>>>> output.o... I will take a look into it.
>>>
>>> Hi Indu,
>>>
>>> I saw another issue in my kernel build with sframes enabled (-Wa,-- 
>>> gsframe):
>>>
>>> ld: warning: orphan section `.init.sframe' from `arch/arm64/kernel/pi/ 
>>> lib-fdt.pi.o' being placed in section `.init.sframe'
>>> [... Many more similar warnings (.init.sframe) ...]
>>>
>>> So, this orphan sections is generated in the build process.
>>>
>>> I am using GNU ld version 2.41-50 and gcc (GCC) 11.4.1
>>>
>>> Is this section needed for sframes to work? or can we discard
>> 
>> No this should not be discarded.  This looks like a wrongly-named but 
>> valid SFrame section.
>> 
>
> Not wrongly named. --prefix-alloc-sections=.init is expected to do that 
> as .sframe is an allocated section.
>

So, these .init.sframe sections are then moved into .sframe by the
linker? (see linker script line below)

Here are some outputs from the build, do they look correct?

One of the objects that were emitting the warning
[ec2-user@...172-31-32-86 linux-upstream]$ readelf -SW arch/arm64/kernel/pi/lib-fdt.pi.o | grep sframe
  [47] .init.sframe      PROGBITS        0000000000000000 003c90 000226 00   A  0   0  8
  [48] .rela.init.sframe RELA            0000000000000000 008f08 000180 18   I 51  47  8

Final vmlinux ELF
[ec2-user@...172-31-32-86 linux-upstream]$ readelf -SW vmlinux | grep sframe
  [ 5] .init.sframe      PROGBITS        ffff80008118c298 119c298 002a88 00   A  0   0  8
  [ 6] .sframe           PROGBITS        ffff80008118ed20 119ed20 247c45 00   A  0   0  8

[ec2-user@...172-31-32-86 linux-upstream]$ readelf --sframe=.sframe vmlinux | head
Contents of the SFrame section .sframe:
  Header :

    Version: SFRAME_VERSION_2
    Flags: SFRAME_F_FDE_SORTED
    Num FDEs: 51842
    Num FREs: 321245

  Function Index :

Does this also look correct?
[ec2-user@...172-31-32-86 linux-upstream]$ readelf --sframe=.init.sframe vmlinux | head
Contents of the SFrame section .init.sframe:
  Header :

    Version: SFRAME_VERSION_2
    Flags: NONE
    Num FDEs: 16
    Num FREs: 50

  Function Index :


and the linker script has this line:

.sframe : AT(ADDR(.sframe) - 0) { __start_sframe_header = .; KEEP(*(.sframe)) __stop_sframe_header = .; }

So, do can you suggest the best way to fix these warnings?

Thanks,
Puranjay


Download attachment "signature.asc" of type "application/pgp-signature" (256 bytes)