Page MenuHomeFreeBSD

Update PPC loader relocations to match the kernel.
Needs RevisionPublic

Authored by jhb on Aug 18 2018, 8:38 PM.

Details

Reviewers
kib
nwhitehorn
Summary
  • Handle all of the same PowerPC relocations in the boot loader that the in-kernel linker handles.
  • Handle all relocations in a PowerPC ELF file loaded by the loader, not just relocations in .data.

Previously, the linker set containing module metadata for the kernel,
and the kernel version number in particular, resided in a separate
section (.data.rel) instead of .data. As a result, the version of the
"kernel" module in the kernel itself was left as zero. When loading a
kernel module from the boot loader, the module failed to initialize
during boot because the it failed the version check on "kernel". The
same module could be loaded fine post-boot via kldload on powerpc,
just not from the loader. By processing all relocations in all sections,
the "kernel" version is now properly populated and modules can be loaded
from the loader.

Tested by: tuexen

Test Plan
  • Michael booted a kernel with a kernel module in loader.conf and it now works where it always failed to boot before.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 18940
Build 18585: arc lint + arc unit

Event Timeline

jhb created this revision.Aug 18 2018, 8:38 PM
jhb added a comment.Aug 18 2018, 8:50 PM

Do we re-process relocations during kernel startup? If so, that explains why kldload worked after boot without this change. However, if so, do we have any kind of policy on which relocations the loader should do? For example, should it really limit itself to a subset to since the kernel will do them all again anyway?

Also, we seem to use the 'only relocate .data' rule on a couple of other architectures in the loader. sparc64 (which powerpc was a copy of), amd64, i386.

Hmmm, reading more code, now I'm really confused. We should probably be honoring the 'data' constraints, as those aren't the .data section, but the bounds of whatever section the SHT_RELA section is relocating. However, in load_elf.c we explicitly avoid doing any relocations on the kernel at all. Michael, when you get back to the console your mac min, can you 1) look at the loader output on the console to see if the printf triggers, and 2) provide a sample 'kernel' file so I can poke at it with readelf? Thanks.

stand/common/reloc_elf.c
226

This is a debugging printf. Michael couldn't see his console until he gets home from BSDCam. I want to verify he sees this as testing my hypothesis that his issue is relocations for sections that aren't .data. I don't intend on including the printf in the commit.

jhb added a comment.Aug 18 2018, 8:51 PM

I should have mentioned that I did look at 'readelf -r' output of the kernel on Michael's box previously and I think it only included a few R_PPC_ADDR32, but they weren't for the relevant section. The rest of the relocations were all R_PPC_RELATIVE which the existing code already handled.

kib added a comment.Aug 18 2018, 9:42 PM

x86 kernels are linked at the fixed address and did not have relocations to be processed either by loader or by kernel itself until recently. With the introduction of ifuncs, there are relocations, but they can only be processed at the kernel bootstrap time since they require calls to the resolver functions, which expect the kernel environment.

Is PPC kernel linked at address different from the final VMA ?

I added Justin and Nathan (who I meant to include from the start).

I believe PPC kernels are relocatable. I think they use ET_DYN instead of ET_EXEC (and there is a pending PR to fix that issue in libkvm as libkvm doesn't work on it currently as it only expects ET_EXEC for a kernel). Justin and/or Nathan can confirm.

In D16794#357383, @jhb wrote:

I added Justin and Nathan (who I meant to include from the start).
I believe PPC kernels are relocatable. I think they use ET_DYN instead of ET_EXEC (and there is a pending PR to fix that issue in libkvm as libkvm doesn't work on it currently as it only expects ET_EXEC for a kernel). Justin and/or Nathan can confirm.

That is correct. PPC kernels are fully relocatable, are very often loaded at addresses different from their link address, may run at addresses different from where they are loaded, and process their relocations completely inside the kernel with no loader involvement.

nwhitehorn requested changes to this revision.Aug 19 2018, 10:58 PM

I just looked at the actual patch. The kernel sometimes (almost always on ppc64, less often on ppc32) relocates itself again post-loader to an address that loader does not know, so I think this does not solve the actual problem, or at least solves it in only a subset of cases. Processing these relocations in the kernel would solve the issue in all cases.

This revision now requires changes to proceed.Aug 19 2018, 10:58 PM
jhb added a comment.Aug 20 2018, 7:58 AM

So we do process all these relocations in the kernel (I just copied the existing in-kernel relocation code into the loader in effect). This definitely fixes loading of modules from the boot loader for tuexen@ which didn't work before. It may be that the kernel is a red herring and that it is his module (tcp_rack.ko) that has relocations that for some reason aren't being handled correctly when loaded from the loader. I'll wait to hear from Michael when he has console access.

tuexen added a comment.EditedAug 20 2018, 11:46 AM

With this patch the kernel can load the tcp_rack module, which is not special, I think.

The tcp_rack module, or any other module I tried, is not loaded via /boot/loader.conf. However, they can be loaded after the system is up via kldload.
I observed that the version meta data in the kernel file looks strange. So the loader can't find it and ends up in
https://svnweb.freebsd.org/base/head/stand/common/load_elf.c?revision=330864&view=markup#l1100
where it adds the module 'kernel' with version 1. When trying to load the tcp_rack module, it can't find a kernel with version 12000?? and
reports that there is no kernel with an appropriate version.

With this patch, I can load the module without any problem by listing it in /boot/loader.conf

Regarding the console messages jhb@ wants to know, I do see
Reloc 22 outside of data, offset 123240
Reloc 22 outside of data, offset 123244
Reloc 22 outside of data, offset 123248
Reloc 22 outside of data, offset 189040
Reloc 22 outside of data, offset 189048
Reloc 22 outside of data, offset 189052
Reloc 22 outside of data, offset 189056
Reloc 22 outside of data, offset 189060
Reloc 22 outside of data, offset 189064
Reloc 22 outside of data, offset 189068
Reloc 22 outside of data, offset 189072
Reloc 22 outside of data, offset 189076
Reloc 22 outside of data, offset 189080
Reloc 22 outside of data, offset 189084
Reloc 22 outside of data, offset 189088
Reloc 22 outside of data, offset 189108
Reloc 22 outside of data, offset 189120
Reloc 22 outside of data, offset 189124
Reloc 22 outside of data, offset 189140
Reloc 22 outside of data, offset 189152
Reloc 22 outside of data, offset 189156
Reloc 22 outside of data, offset 189168
Reloc 22 outside of data, offset 189172
Reloc 22 outside of data, offset 189184
Reloc 22 outside of data, offset 189188
Reloc 22 outside of data, offset 189192
Reloc 22 outside of data, offset 189196
Reloc 22 outside of data, offset 189232
Reloc 6 outside of data, offset 38174

jhb added a comment.Aug 29 2018, 8:25 PM

Michael, I would like to get a copy of your kernel and tcprack.ko when you get some spare time.

Nathan, this definitely changes the behavior for Michael. In particular, the loader wants to scan all the MODULE_VERSION metadata entries itself to build its own table to check MODULE_DEPEND lines in the kernel and modules itself. I think the issue here is that the kernel has a relocation that actually adjusts the pointers in the MODULE_VERSION storage for the kernel and without the relocations processed, the loader can't properly build it's metadata for the "kernel" module. It may be that the kernel will relocate to a different base address when it starts, but this change is about letting the loader see the module info.

kib added a comment.Aug 29 2018, 9:15 PM
In D16794#361490, @jhb wrote:

Nathan, this definitely changes the behavior for Michael. In particular, the loader wants to scan all the MODULE_VERSION metadata entries itself to build its own table to check MODULE_DEPEND lines in the kernel and modules itself. I think the issue here is that the kernel has a relocation that actually adjusts the pointers in the MODULE_VERSION storage for the kernel and without the relocations processed, the loader can't properly build it's metadata for the "kernel" module. It may be that the kernel will relocate to a different base address when it starts, but this change is about letting the loader see the module info.

I am not sure about details of PPC relocations, but I suspect that loaders relocations should be undone if kernel is loaded at different VA and processes the relocations one more time. At least, this is the case for x86.

In D16794#361490, @jhb wrote:

Michael, I would like to get a copy of your kernel and tcprack.ko when you get some spare time.

I will see how to get this to you. I did a buildworld / installworld yesterday with the result that the system does not boot anymore.
This is NOT related to the change of this review. I posted the error message on the ppc mailing list...

jhb added a comment.Aug 29 2018, 10:11 PM

I believe since ppc is using Elf_Rela and that all of the relocations calculated absolute values to store at *where or *hwhere (vs using += or the like), the loader relocations just get overwritten by the kernel. I think when using Elf_Rel you might use the original value of *where as the addend in which case multiple passes of relocations is indeed destructive, but I don't think it is for Elf_Rela.

In D16794#361523, @jhb wrote:

I believe since ppc is using Elf_Rela and that all of the relocations calculated absolute values to store at *where or *hwhere (vs using += or the like), the loader relocations just get overwritten by the kernel. I think when using Elf_Rel you might use the original value of *where as the addend in which case multiple passes of relocations is indeed destructive, but I don't think it is for Elf_Rela.

This is a correct diagnosis. We actually rely on this: On many 64-bit PPC systems, the kernel will relocate itself multiple times during startup before it gets comfortable in a fixed address range.

jhb added a comment.Aug 29 2018, 11:48 PM

On closer examination, the loader doesn't actually modify the kernel or modules in place at all. Instead, this routine is used to apply relocations on copies of data. So for example, the code to parse module metadata copies individual records out of the kernel into a local variable on the stack, then asks the relocation to be applied to that local copy. This is a really good reason to honor the the 'data' and 'len' bounds as otherwise it allows writing to random crap on the stack. Also, we do explicitly ignore all relocations for the kernel itself, so it must be some relocation in tcp_rack.ko that wasn't handled before that is being handled now perhaps. I would have expected that to trigger a warning if so in the existing code though for an "unhandled relocation type".

In D16794#361563, @jhb wrote:

On closer examination, the loader doesn't actually modify the kernel or modules in place at all. Instead, this routine is used to apply relocations on copies of data. So for example, the code to parse module metadata copies individual records out of the kernel into a local variable on the stack, then asks the relocation to be applied to that local copy. This is a really good reason to honor the the 'data' and 'len' bounds as otherwise it allows writing to random crap on the stack. Also, we do explicitly ignore all relocations for the kernel itself, so it must be some relocation in tcp_rack.ko that wasn't handled before that is being handled now perhaps. I would have expected that to trigger a warning if so in the existing code though for an "unhandled relocation type".

Yeah, that's a weird one. One caution I just wanted to repeat from earlier is that, because of various shenanigans like the double-relocation I mentioned earlier, we *really* need loader to just load blobs. If the kernel is relying on the addresses from loader, or addresses in the kernel or modules, meaning something as raw numbers, things *will* collapse later on.

Hi John,
here is the TCP module


and here is the kernel

Best regards
Michael

linimon removed a subscriber: linimon.May 10 2019, 3:56 PM