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.

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.

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.

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.

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

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.

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...

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.

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