lkrg-org / lkrg

Linux Kernel Runtime Guard
https://lkrg.org
Other
410 stars 72 forks source link

False positive detection likely caused by aggressive inlining in custom kernel build #166

Open sempervictus opened 2 years ago

sempervictus commented 2 years ago

image ^^ that's on the 5.10.101 build which has all of the exports marked as suggested (no errors at load-time anyway).

solardiz commented 2 years ago

Nasty. This illustrates another rather fundamental issue we're facing lately: even when we do find and hook the symbol, there might be an inlined copy (or several) of the same code as well, and we wouldn't hook that, resulting in inconsistent LKRG state (it was supposed to be updated by all uses of the function, but is in fact getting updated by only some or possibly even not updated at all if the function/symbol we forced the compiler to preserve with EXPORT_SYMBOL became dead code and not code the rest of the kernel actually calls rather than inlines).

No easy fix, sorry!

solardiz commented 2 years ago

In the case of your kernel build, it looks like we have this problem for at least two symbols, and I guess at least one of those wasn't among the forced EXPORT_SYMBOL ones (not in add-exports.sh).

sempervictus commented 2 years ago

Any idea what the missing symbol might be so i could add it to the script? Doesnt sound like it'll help the problem you describe, but at least might as well squash one bug while we're here.

solardiz commented 2 years ago

No, I don't think we should be adding one more symbol to the exports because of this, nor would it be expected to help.

My current best idea for a workaround is to be adding noinline (Linux kernel specific macro) or __attribute__((__noinline__)) to right before every function that we hook and that may potentially be inlined, which you can do with sed. You can test this approach by doing it for the two seccomp-related symbols - then see if the seccomp-related false positive that killed (ogrotate) (why that weird name?) on your system would be gone.

This would be instead of adding the exports - we don't actually rely on the exports anyway, looking those symbols up in a hackish way instead - we just need the functions to be preserved as such and not inlined.

Adam-pi3 commented 2 years ago

@solardiz Adding __attribute__((__noinline__)) should help. In theory, EXPORT_SYMBOL can't be inline so this should work too. However, I remember that it was not always the case and compiler could generate unpredictable results:

https://lore.kernel.org/lkml/1368086323-9412-2-git-send-email-yefremov.denis@gmail.com/

solardiz commented 2 years ago

My understanding is that exporting a symbol takes and records its address, causing a non-inlined copy of the function to be preserved, but it does not prevent the compiler from also generating inlined copies of the function's body or its parts. Explicit noinline should achieve the latter.