CONFIG_SECCOMP panic with LLD

[nathanchance]

Forked off from ClangBuiltLinux/continuous-integration#81

With my fork of the Pi kernel, I can successfully build, boot, and run a kernel compiled with Clang on both 4.14 and 4.20. Additionally, thanks to the top two patches, I can link the kernel successfully with lld. However, it panics before getting to userspace. I have pulled a log, which I am happy to triage as needed but honestly, I don't know where to start.

[nickdesaulniers]

The oops' all look the same

[    2.401795] PC is at 0x80008274
[    2.401812] LR is at do_seccomp+0x138/0x73c

so ARM always has a dedicated register for its parent stack frame, "the link register," LR. So LR is do_seccomp+0x138 meaning if you disassemble the object file from the source file that defines do_seccomp (use grep or ag to find which translation unit it comes from), then look at the instruction at 0x138 bytes from the start of do_seccomp label, it's probably making a call into another function (on ARM, a call instruction is bl "branch and link" which I think of as call+set LR). So do_seccomp is jumping to 0x80008274. Looks like the kernels unwinder wasn't able to tell you what that is.

I don't know what seccomp is, and haven't looked, but maybe LLD is broken with seccomp? Maybe if seccomp can be disabled, you might be able to boot, then we can go from there.

[nathanchance]

Thank you very much for that analysis! Disabling CONFIG_SECCOMP allows it to boot successfully and everything appears to be functioning normally (been running for about five minutes without issues). I'll re-enable CONFIG_SECCOMP and poke my head around the disassembly and see what it looks like.

[nathanchance]

I wasn't able to fully dive into my analysis tonight since I didn't have all the time I was hoping for but I uploaded the full disassembly of kernel/seccomp.o and a new stacktrace in case someone wants to glance them over.

https://gist.github.com/75872ee94ec8cbea55e279ab32f4d8e2
https://gist.github.com/7af41fcbb2f804a5f65bbedb81a1462a

It did occur to me that @kees is the seccomp maintainer, he may have some insight into this (although, there's no guarantee that this is isolated to seccomp, it could be a global issue that is just happening with seccomp in this config).

[nickdesaulniers]

Yes @kees can help then. Next question, is this isolated to arm32? I assume it's maybe not in any defconfig, so we don't have CI coverage of it?

[nickdesaulniers]

Thanks for the disassembly. Because it's the disassembly of an object file that hasn't been linked yet, the addresses are not fully resolved yet, so the addresses from the stack trace don't quite line up with the disassembled object file. That's ok.

[    2.377552] LR is at do_seccomp+0x1a8/0x768

(note, this is different from the first trace, took me a few minutes to figure that out, screws up the below math)

000004f8 <do_seccomp>:

0x1a8 + 0x4f8 == 0x6a0

     6a0:	e3500000 	cmp	r0, #0

Now another special thing is that the LR points to the next instruction after the child stack frame returns, we need to subtract 4 bytes (1 word on arm32).

0x6a0 - 0x1 == 0x69c

     69c:	ebfffffe 	bl	0 <arm_copy_from_user>

aha, there's a branch and link instruction (bl) which would have set the LR to 0x6a0. Looks like arm_copy_from_user is called twice from do_seccomp, why does this second one fail? Why is it special? do_seccomp might not immediately call arm_copy_from_user, but through inlining it may be the case.

Next idea to try; can you find the two invocations of arm_copy_from_user (or maybe copy_from_user) in do_seccomp? If you comment out one invocation and maybe return early, or jump to some error handler, or put some dummy value in the variable it's trying to read into, does booting progress? (We're trying to isolate the problematic source)

[nickdesaulniers]

Also, I'll bet that arm_copy_from_user is implemented in assembly, which may be why it doesn't show up next to the PC in the unwind (I'm not sure how that's fixed, honestly).

[nathanchance]

Next question, is this isolated to arm32? I assume it's maybe not in any defconfig, so we don't have CI coverage of it?

CONFIG_SECCOMP=y is in multi_v7_defconfig and arm64 defconfig so I guess real hardware is exploiting this in a way QEMU isn't. I can't say if this is isolated to just arm32 because I don't have an arm64 board.

Also, I'll bet that arm_copy_from_user is implemented in assembly, which may be why it doesn't show up next to the PC in the unwind (I'm not sure how that's fixed, honestly).

Correct: https://github.com/nathanchance/pi-kernel/blob/rpi-4.14.y/arch/arm/lib/copy_from_user.S

aha, there's a branch and link instruction (bl) which would have set the LR to 0x6a0. Looks like arm_copy_from_user is called twice from do_seccomp, why does this second one fail? Why is it special? do_seccomp might not immediately call arm_copy_from_user, but through inlining it may be the case.

Yeah that's interesting... I see three instances of copy_from_user (which are turned into arm_copy_from_user) and none of them are directly in do_seccomp so inlining must have happened.

https://github.com/nathanchance/pi-kernel/blob/rpi-4.14.y/kernel/seccomp.c#L422
https://github.com/nathanchance/pi-kernel/blob/rpi-4.14.y/kernel/seccomp.c#L428
https://github.com/nathanchance/pi-kernel/blob/rpi-4.14.y/kernel/seccomp.c#L909

Since CONFIG_COMPAT isn't relevant for a 32-bit kernel, it must be the last two.

Next idea to try; can you find the two invocations of arm_copy_from_user (or maybe copy_from_user) in do_seccomp? If you comment out one invocation and maybe return early, or jump to some error handler, or put some dummy value in the variable it's trying to read into, does booting progress? (We're trying to isolate the problematic source)

The problematic one appears to be in in seccomp_prepare_user_filter, as I can successfully boot with this diff (although my modules didn't load so I wasn't able to verify the version string successfully):

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 075e344a87c3..6e243d9886dc 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -425,9 +425,11 @@ seccomp_prepare_user_filter(const char __user *user_filter)
                fprog.filter = compat_ptr(fprog32.filter);
        } else /* falls through to the if below. */
 #endif
+#if 0
        if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
                goto out;
        filter = seccomp_prepare_filter(&fprog);
+#endif
 out:
        return filter;
 }

[nickdesaulniers]

CONFIG_SECCOMP=y is in multi_v7_defconfig and arm64 defconfig so I guess real hardware is exploiting this in a way QEMU isn't.

That would be bad, and peculiar if something like that was the linker's fault. But the Pi's config is not the same as our defconfigs (and we should verify booting the pi kernel in qemu).

CONFIG_MMU changes the implementation of raw_copy_from_user. Is that set for our QEMU tested defconfigs, but not for the pi, or vice-versa? (I feel like that would have blown up in other ways, and it's not clear to my why this lone call site is problematic).

[nathanchance]

and we should verify booting the pi kernel in qemu

Yeah this doesn't really seem to easy (at least with their config). Doing some research, it seems like everyone is using this repo and its build script uses arch/arm/configs/versatile_defconfig as a base, not arch/arm/configs/bcm2709_defconfig (because it's only concerned with running RPI userspace, not its kernel).

I will try to do more research on this tomorrow and I'll double check all of the results I've posted so far since I agree that it's super bizarre that JUST changing the linker is resulting in this issue...

CONFIG_MMU changes the implementation of raw_copy_from_user. Is that set for our QEMU tested defconfigs, but not for the pi, or vice-versa? (I feel like that would have blown up in other ways, and it's not clear to my why this lone call site is problematic).

It is default y on both arm and arm64 so I assume QEMU should be using it.

[kees]

Does this fail under qemu too? Have you compared the disassemble between ld.bfd and ld.lld?

There is nothing "special" about this copy_from_user() that I can see.

It looks like the Oops is at address 8000000d -- which seems like a strange address: it's the kernel base plus 14, which is pretty small... I would assume there is a dereference from some structure address but the address is very wrong. Perhaps the linker failed to apply the right offset, and used only the kernel base offset?