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

Song Liu
2025-02-20T04:50:21Z

On Wed, Feb 19, 2025 at 9:44 AM Song Liu <song@kernel.org> wrote:
>
> On Tue, Feb 18, 2025 at 10:41 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Tue, Feb 18, 2025 at 10:20:10AM -0800, Song Liu wrote:
> > > Hi Josh,
> > >
> > > On Mon, Feb 17, 2025 at 10:37 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > >
> > > > On Mon, Feb 17, 2025 at 08:38:22PM -0800, Song Liu wrote:
> > > > > On Fri, Feb 14, 2025 at 3:23 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > > > Poking around the arm64 module code, arch/arm64/kernel/module-plts.c
> > > > > > is looking at all the relocations in order to set up the PLT. That also
> > > > > > needs to be done for klp relas, or are your patches already doing that?
> > > > >
> > > > > I don't think either version (this set and my RFC) added logic for PLT.
> > > > > There is some rela logic on the kpatch-build side. But I am not sure
> > > > > whether it is sufficient.
> > > >
> > > > The klp relas looked ok. I didn't see any signs of kpatch-build doing
> > > > anything wrong. AFAICT the problem is that module-plts.c creates PLT
> > > > entries for regular relas but not klp relas.
> > >
> > > In my tests (with printk) module-plts.c processes the .
> > > klp.rela.vmlinux..text.copy_process section just like any other .rela.*
> > > sections. Do we need special handling of the klp.rela.* sections?
> >
> > Ok, I see how it works now:
> >
> > klp_write_section_relocs()
> > apply_relocate_add()
> > module_emit_plt_entry()
> >
> > If that code is working correctly then I'm fresh out of ideas...
>
> I tried to dump assembly of copy_process, but couldn't find any
> clue. I am wondering whether this is an issue with gcc-14.2.1.
>
> Puranjay, could you please try with a different gcc, like some
> version of gcc-14?

I figured out why this is broken with gcc-14.

The stack trace points to tty_kref_get(), which is eventually
a __refcount_add(). With gcc, there are multiple of constprop
versions of the function.

$ readelf -s -W vmlinux | grep __refcount_add
19747: ffff800080872eb0 104 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0
20153: ffff80008001fcd8 88 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0
36428: ffff800080e62ac0 100 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0
45055: ffff8000800bf130 100 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0
54823: ffff8000801c2a40 100 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0
180848: ffff8000810a8cb0 100 FUNC LOCAL DEFAULT 2
__refcount_add.constprop.0

The problem is, with gcc-14, these symbols are NOT sorted by
their addresses. OTOH, the order in kallsyms is like:

[root@(none) /test-klp]# grep __refcount_add /proc/kallsyms
ffff80008001fcd8 t __refcount_add.constprop.0
ffff8000800bf130 t __refcount_add.constprop.0
ffff8000801c2a40 t __refcount_add.constprop.0
ffff800080872eb0 t __refcount_add.constprop.0
ffff800080e62ac0 t __refcount_add.constprop.0
ffff8000810a8cb0 t __refcount_add.constprop.0

kpatch-build uses readelf to get the symtab, and then calculate
proper sympos based on this symtab. However, this sympos
is different when the kernel applies this KLP. In this case,
copy_process calls ffff8000800bf130. kpatch-build gives
sympos of 4, based on the output of readelf, but ffff8000800bf130
is actually at position 2 in kallsyms.

gcc-11, OTOH, doesn't have this problem with readelf.

Indu, is this behavior (symbols with same name are not in
sorted order from readelf -s) expected? Or is this a bug?
I am using this gcc:

$ gcc --version
gcc (GCC) 14.2.1 20240801 (Red Hat 14.2.1-1)
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Thanks,
Song