On Mon, Sep 25, 2023 at 1:03 PM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 2023/09/24 1:06, KP Singh wrote:
> >> I was not pushing LKM-based LSM because the LSM community wanted to make it possible to
> >> enable arbitrary combinations (e.g. enabling selinux and smack at the same time) before
> >> making it possible to use LKM-based LSMs.
> (...snipped...)
> >> As a reminder to tell that I still want to make LKM-based LSM officially supported again,
> >> I'm responding to changes (like this patch) that are based on "any LSM must be built into
> >> vmlinux". Please be careful not to make changes that forever make LKM-based LSMs impossible.
>
> You did not recognize the core chunk of this post. :-(
>
> It is Casey's commitment that the LSM infrastructure will not forbid LKM-based LSMs.
> We will start allowing LKM-based LSMs. But it is not clear how we can make it possible to
> allow LKM-based LSMs.
I think this needs to be discussed if and when we allow LKM based LSMs.
>
> Suppose you replace the linked list (which does not need to limit number of LSMs activated)
> with static calls (which limits number of LSMs activated, due to use of compile-time determined
> MAX_LSM_COUNT value at
>
> struct lsm_static_calls_table {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> struct lsm_static_call NAME[MAX_LSM_COUNT];
> #include <linux/lsm_hook_defs.h>
> #undef LSM_HOOK
> } __randomize_layout;
>
> . If NAME[MAX_LSM_COUNT] were allocated like
>
> NAME = kcalloc(sizeof(struct lsm_static_call), number_of_max_lsms_to_activate, GFP_KERNEL | __GFP_NOFAIL);
>
> (where number_of_max_lsms_to_activate is controlled using kernel command line parameter)
> rater than
>
> struct lsm_static_call NAME[MAX_LSM_COUNT];
>
> , it is easy to allow LKM-based LSMs.
>
One needs to know MAX_LSM_COUNT at compile time (not via kernel
command line), I really suggest you try out your suggestions before
posting them. I had explained this to you earlier, you still chose to
ignore and keep suggesting stuff that does not work.
https://lore.kernel.org/bpf/CACYkzJ7Dn=W1Kd5M_bXOzoomzdjMXBoEZZo5k=cgQ4R6f5G+vw@xxxxxxxxxxxxxx/
It is used in the preprocessor to generate the static calls, it cannot
come from the command line.
> But if NAME[MAX_LSM_COUNT] is allocated in a way which cannot be expanded using kernel
> command line parameter (this is what "[PATCH v3 2/5] security: Count the LSMs enabled
> at compile time" does), how can the LKM-based LSMs be registered? Introduce a LSM module
> which revives the linked list and registration function (which this patch tried to remove) ?
> If yes, do we want to use
>
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
>
> for built-in LSMs and a different macro for LKM-based LSMs?
>
> Do we want/agree to manage two different set of macros/functions only for handling
> both built-in LSMs and loadable LSMs?
We will see when this happens. I don't think it's a difficult problem
and there are many ways to implement this:
* Add a new slot(s) for modular LSMs (One can add up to N fast modular LSMs)
* Fallback to a linked list for modular LSMs, that's not a complexity.
There are serious performance gains and I think it's a fair trade-off.
This isn't even complex.
Now, this patch and the patch that makes security_hook_heads
__ro_after_init by removing CONFIG_SECURITY_HOOKS_WRITABLE breaks your
hack. But that hack (https://akari.osdn.jp/1.0/chapter-3.html.en) is
unsupported.
>
> That's a lot of complication, compared to temporarily making the security_hook_heads writable.
No, that's not complicated. All I can say is, when the time comes,
and if the community agrees on LMK based modules, this patch won't
make it any difficult or easy. There are many implementations, even
this patch, that can provide LKM based LSMs API (but hacks will be
hard, sure!)
- KP
>
>
>
> > You are trying to use an unexported symbol from the module with lots
> > of hackery to write to be supported and bring it up in a discussion?
> > Good luck!
>
> Currently LKM-based LSMs is not officially supported. But LKM-based LSMs will become
> officially supported in the future. Therefore, I respond to any attempt which tries
> to make LKM-based LSMs impossible.
>
> >
> > Regardless, if what you are doing really works after
> > https://lore.kernel.org/all/20200107133154.588958-1-omosnace@xxxxxxxxxx,
> > then we need to fix this as the security_hook_heads should be
> > immutable after boot.
>
> You should learn how the __ro_after_init works. I will throw NACK if someone tries
> to add an exception to __ro_after_init handling before we make it possible to allow
> LKM-based LSMs.
>
