lkrg-org / lkrg

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

P_LKRG_UNHIDE's behavior inconsistent with its name #251

Open solardiz opened 1 year ago

solardiz commented 1 year ago

We wrap some functionality related to the hide feature/setting in #ifdef P_LKRG_UNHIDE and define that macro by default. Changing that default (not defining the macro) should probably remove the ability to unhide. What it actually does is remove the code to unhide and the hide setting, which remains disabled by default, so in essence it removes the ability to hide.

For the existence of P_LKRG_UNHIDE to make sense, undefining it should probably preserve the hide setting, but disallow disabling it (once enabled). Alternatively, hide should be enabled by default (at least in builds without P_LKRG_UNHIDE).

solardiz commented 1 year ago

The sysctl setting used to be unhide prior to e5924c419a9565b940a67cc169ad3ba0ebc1f975 in 2017, so prior to public release of LKRG. Changing it to hide back then is what made P_LKRG_UNHIDE inconsistent with its name. The easiest fix for now would be to also rename P_LKRG_UNHIDE to P_LKRG_HIDE, but I'm not sure that's what we want long-term.

solardiz commented 1 year ago

Disabling P_LKRG_UNHIDE currently leaves p_hide_itself compiled in, and there's a call from p_lkrg_register:

   if (P_CTRL(p_hide_lkrg)) {
      p_hide_itself();
   }

However, there's nothing to enable that setting in such a build. So we have dead code. If we rename to P_LKRG_HIDE, we should also exclude that dead code in builds with P_LKRG_HIDE disabled.

Alternatively, we can add a way to set p_hide_lkrg in such builds - add a module parameter or/and make it default to enabled.