dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

kpatch-build error: can't create dynrela for section .rela.static_call_sites #1272

Closed liu-song-6 closed 2 years ago

liu-song-6 commented 2 years ago

We are seeing the following error from kpatch-build

space-info.o: changed function: __reserve_bytes
/data/users/songliubraving/kpatch/kpatch-build/create-diff-object: ERROR: space-info.o: kpatch_create_intermediate_sections: 3362: can't create dynrela for section .rela.static_call_sites (symbol __SCK__tp_func_btrfs_done_preemptive_reclaim): no bundled or section symbol

The patch to repro this error can be very simple:

diff --git i/fs/btrfs/space-info.c w/fs/btrfs/space-info.c
index e692c880deb7..ae7497b430e9 100644
--- i/fs/btrfs/space-info.c
+++ w/fs/btrfs/space-info.c
@@ -1489,6 +1489,8 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
        int ret = 0;
        bool pending_tickets;

+       if (IS_ERR(fs_info))
+               return 0;
        ASSERT(orig_bytes);
        ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL);

After digging into this, we found this happens because missing symbol name for the .static_call_sites section. We need to access static_call_sites section for the tracepoint call trace_btrfs_done_preemptive_reclaim. The symbol name for static_call_sites is generated by the following step in scripts/Makefile.build:

cmd_modversions_c =                                                             \
        if $(OBJDUMP) -h $@ | grep -q __ksymtab; then                           \
                $(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))  \
                    > $(@D)/.tmp_$(@F:.o=.ver);                                 \
                                                                                \
                $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@               \
                        -T $(@D)/.tmp_$(@F:.o=.ver);                            \
                mv -f $(@D)/.tmp_$(@F) $@;                                      \
                rm -f $(@D)/.tmp_$(@F:.o=.ver);                                 \
        fi
endif

As space-info.c doesn't export any symbol, space-info.o doesn't have any __ksymtab, and we skipped cmd_gensymtypes_c.

I tried a few things to fix this issue, but haven't figured out a good solution. Any suggestions on what would be the best fix here?

Thanks!

jpoimboe commented 2 years ago

Recent toolchains only create a section symbol if it's needed, i.e. if there's a reference to it. I think the fix here would be to just create the section symbol, something like so (untested)

diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c
index bebe3bd..d71259b 100644
--- a/kpatch-build/create-diff-object.c
+++ b/kpatch-build/create-diff-object.c
@@ -3355,12 +3355,23 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf,

            /* add rela to fill in krelas[index].dest field */
            ALLOC_LINK(rela2, &krela_sec->rela->relas);
-           if (relasec->base->secsym)
-               rela2->sym = relasec->base->secsym;
-           else
-               ERROR("can't create dynrela for section %s (symbol %s): no bundled or section symbol",
-                     relasec->name, rela->sym->name);
+           if (!relasec->base->secsym) {
+               struct symbol *sym;

+               /*
+                * Newer toolchains are stingy with their
+                * section symbols, create one if it doesn't
+                * exist already.
+                */
+               ALLOC_LINK(sym, &kelf->symbols);
+               sym->sec = relasec->base;
+               sym->sym.st_info = GELF_ST_INFO(STB_LOCAL, STT_SECTION);
+               sym->type = STT_SECTION;
+               sym->bind = STB_LOCAL;
+               sym->name = relasec->base->name;
+               relasec->base->secsym = sym;
+           }
+           rela2->sym = relasec->base->secsym;
            rela2->type = absolute_rela_type(kelf);
            rela2->addend = rela->offset;
            rela2->offset = (unsigned int)(index * sizeof(*krelas) + \
jpoimboe commented 2 years ago

I was able to recreate, and I verified the above patch fixed it. Will make a pull request.

liu-song-6 commented 2 years ago

Yes, this works great, thanks!