Re: [PATCH] init/main.c: Initialize early LSMs after arch code
From: Guenter Roeck
Date: Tue Aug 13 2024 - 00:08:07 EST
On 8/12/24 15:02, KP Singh wrote:
On Mon, Aug 12, 2024 at 11:33 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
On Mon, Aug 12, 2024 at 5:14 PM KP Singh <kpsingh@xxxxxxxxxx> wrote:
On Mon, Aug 12, 2024 at 9:33 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
On Mon, Aug 12, 2024 at 1:12 PM KP Singh <kpsingh@xxxxxxxxxx> wrote:
JFYI, I synced with Guenter and all arch seem to pass and alpha does
not work due to a reason that I am unable to debug. I will try doing
more debugging but I will need more alpha help here (Added the
maintainers to this thread).
Thanks for the update; I was hoping that we might have a resolution
for the Alpha failure by now but it doesn't look like we're that
lucky. Hopefully the Alpha devs will be able to help resolve this
without too much trouble.
Unfortunately, this does mean that I'm going to drop the static call
patches from the lsm/dev branch so that we can continue merging other
things. Of course this doesn't mean the static call patches can't
come back in later during this dev cycle once everything is solved if
there is still time, and worst case there is always the next dev
cycle.
Do we really want to drop them for alpha? I would rather disable
CONFIG_SECURITY for alpha and if people really care for alpha we can
enable it. Alpha folks, what do you think?
Seriously? I realize Alpha is an older, lesser used arch, but it is
still a supported arch and we are not going to cause a regression for
the sake of a new feature. As I mentioned earlier, once the problem
is resolved we can bring the patchset back into lsm/dev; if it gets
resolved soon enough we can even do it during this dev cycle.
Okay, more data for the alpha folks, when I moved trap_init() before
early_security_init() everything seemed to work, I think we might need
to call trap_init() from setup_arch and this would fix the issue. As
to why? I don't know :)
Would alpha folks be okay with this patch:
kpsingh@kpsingh:~/projects/linux$ git diff
diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index bebdffafaee8..53909c1be4cf 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -657,6 +657,7 @@ setup_arch(char **cmdline_p)
setup_smp();
#endif
paging_init();
+ trap_init();
}
and provide me some reason as to why this works, it would be great for
a patch description
Your code triggers a trap (do_entUna, unaligned access) which isn't handled unless
trap_init() has been called before.
Reason is that static_calls_table is not 8-byte aligned, causing the unaligned
access in:
static void __init lsm_static_call_init(struct security_hook_list *hl)
{
struct lsm_static_call *scall = hl->scalls;
int i;
for (i = 0; i < MAX_LSM_COUNT; i++) {
/* Update the first static call that is not used yet */
if (!scall->hl) { <-- here
__static_call_update(scall->key, scall->trampoline,
hl->hook.lsm_func_addr);
scall->hl = hl;
static_branch_enable(scall->active);
return;
}
scall++;
}
panic("%s - Ran out of static slots.\n", __func__);
}
A somewhat primitive alternate fix is:
diff --git a/security/security.c b/security/security.c
index aa059d0cfc29..dea9736b2014 100644
--- a/security/security.c
+++ b/security/security.c
@@ -156,7 +156,7 @@ static __initdata struct lsm_info *exclusive;
* and a trampoline (STATIC_CALL_TRAMP) which are used to call
* __static_call_update when updating the static call.
*/
-struct lsm_static_calls_table static_calls_table __ro_after_init = {
+struct lsm_static_calls_table static_calls_table __ro_after_init __attribute__((aligned(8))) = {
#define INIT_LSM_STATIC_CALL(NUM, NAME) \
(struct lsm_static_call) { \
.key = &STATIC_CALL_KEY(LSM_STATIC_CALL(NAME, NUM)), \
Guenter