Page MenuHomeFreeBSD

Add uuid for module to prevent incompetable coredump loaded by debugger
Needs ReviewPublic

Authored by aokblast on Sat, May 11, 7:08 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 18, 7:18 PM
Unknown Object (File)
Fri, May 17, 3:42 AM
Unknown Object (File)
Thu, May 16, 6:56 PM
Unknown Object (File)
Thu, May 16, 5:42 PM
Unknown Object (File)
Thu, May 16, 4:22 PM
Unknown Object (File)
Tue, May 14, 7:20 PM
Unknown Object (File)
Mon, May 13, 7:51 AM
Unknown Object (File)
Sun, May 12, 11:49 AM
Subscribers

Details

Summary

This feature is aim at preventing debugger to load incompatible kernel dump with corresponding module. The mechanism behind it is to expose the address of uuid to debugger and let the debugger check if the dump has the same uuid with the binary file. If they are same, it can be loaded successfully. If not, even though we can load it, the debug information may be wrong.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 57664
Build 54552: arc lint + arc unit

Event Timeline

aokblast added reviewers: emaste, markj, kib.
aokblast added a reviewer: lwhsu.
aokblast edited the summary of this revision. (Show Details)
  • Add uuid for module to prevent incompetable coredump loaded by debugger
  • Expose offset of uuid_addr and uuid_size in linker_file for debugger

It probably makes sense to split the change into two: one for loader, one for kernel.

Don't we need to ensure that notes are preserved, in the modules linker scripts? (sys/conf/ldscript.kmod.ARCH)

I assume that the debugger changes as well as adding UUIDS to the built modules would happen somewhere else?

stand/common/load_elf.c
746 ↗(On Diff #138392)

Indent should be +4 spaces for the continuation line

sys/kern/link_elf.c
1319

Unindent by 4 spaces back

sys/kern/link_elf_obj.c
608

unindent by -4 spaces

In D45161#1030112, @kib wrote:

It probably makes sense to split the change into two: one for loader, one for kernel.

Don't we need to ensure that notes are preserved, in the modules linker scripts? (sys/conf/ldscript.kmod.ARCH)

I assume that the debugger changes as well as adding UUIDS to the built modules would happen somewhere else?

  1. Oh, I will separate it into two diffs.
  2. I think ldscript already guarantee that. Besides, I decide to skip this check in LLDB with waring printed as a fallback.
  3. Yes, I will write and send the patch in here as an extension of the previous work.

Thinking about it more, why does the loader change needed at all? Kernel parses/should parse the elf.

For preloaded relocatable kernel module, I think the modification of loader is necessary as I discover SHT_NOTE section was not loaded by boot1 originally when the kernel module is preloaded.
For shared object, actually we can get the address of uuid when kernel is loaded. I modify the loader because I discover that CTORS pass the information from loader and I think I should follow it?

This comment was removed by aokblast.

For preloaded relocatable kernel module, I think the modification of loader is necessary as I discover SHT_NOTE section was not loaded by boot1 originally when the kernel module is preloaded.

This is most likely can/should be fixed by linker script.

For shared object, actually we can get the address of uuid when kernel is loaded. I modify the loader because I discover that CTORS pass the information from loader and I think I should follow it?

Let me reformulate my point: this functionality (uuid module identification) should not depend on updated loader. I do not see why do we need to establish such requirement.

I understand your point, I will try not to modify the loader.

After my investigation, I discover that shstrtab section is not loaded into memory (because no SHF_ALLOC). Thus I cannot compare the section name with ".note.gnu.build-id". Besides, in link_elf_link_preload, the function don't open the file as link_elf_load_file but use all of the memory and preload information from boot1. Thus if I want to compare the section name, I should load the file, create temp memory space for it, and read file. Does it worth to a file in kernel just for compare a section name?

Where does the UUID come from? Is it just the build-id?

LLDB refer build-id as UUID. Sorry about the imprecise word.

@aokblast
I'm not familiar with debuggers, so this might be a silly question.

@emaste made a commit 74cd06b42ea6 which will write note.gnu.build-id section to kernel / kernel module files. Can that ( note.gnu.build-id ) serve the same purpose aim at preventing debugger to load incompatible kernel dump with corresponding module ?

An example of note.gnu.build-id on 13.3:

# freebsd-version -k
13.3-RELEASE-p1

# readelf -n /boot/kernel/if_bridge.ko 

Notes at offset 0x0000b798 with length 0x00000024:
  Owner         Data size       Description
  GNU           0x00000014      NT_GNU_BUILD_ID (Build id set by ld(1))
   Build ID: ab49d4bfb1a1ed65193083dc7f7eb653a65fad0d

# readelf -n /boot/kernel/if_lagg.ko

Notes at offset 0x0001324c with length 0x00000024:
  Owner         Data size       Description
  GNU           0x00000014      NT_GNU_BUILD_ID (Build id set by ld(1))
   Build ID: 90a012c146bc5a6e1dd3d04358f39fc947b521ea

And on 14.0

# freebsd-versioin -k
14.0-RELEASE-p6

# readelf -n /boot/kernel/if_lagg.ko

Notes at offset 0x0001359c with length 0x00000024:
  Owner         Data size       Description
  GNU           0x00000014      NT_GNU_BUILD_ID (Build id set by ld(1))
   Build ID: 934128ad28a2562d0c1dcedbaa067d9fb93d27de

Sorry about my late reply. The patch Ed commit is the pre-request to ensure my patch work. The logic of my patch is the loader would load the build-id from elf file in .gnu.note.build-id section into memory. When kernel crashed, it generate coredump and the build-id loaded from the loader will dump into the coredump file at the same time. After that, when debugger load kernel and it's coredump, the debugger will try to find the elf file of module and its debug file. We can compare the build-id from memory with elf file. If they are different, we shouldn't load the debug file (because they are incompatible).