On Tue, Jun 11, 2024 at 6:35 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On May 15, 2024 KP Singh <kpsingh@xxxxxxxxxx> wrote:
> >
> >
[...]
> > +/**
> > + * security_toggle_hook - Toggle the state of the LSM hook.
> > + * @hook_addr: The address of the hook to be toggled.
> > + * @state: Whether to enable for disable the hook.
> > + *
> > + * Returns 0 on success, -EINVAL if the address is not found.
> > + */
> > +int security_toggle_hook(void *hook_addr, bool state)
> > +{
> > + struct lsm_static_call *scalls = ((void *)&static_calls_table);
>
> GCC (v14.1.1 if that matters) is complaining about casting randomized
> structs. Looking quickly at the two structs, lsm_static_call and
> lsm_static_calls_table, I suspect the cast is harmless even if the
> randstruct case, but I would like to see some sort of fix for this so
> I don't get spammed by GCC every time I do a build. On the other hand,
> if this cast really is a problem in the randstruct case we obviously
> need to fix that.
>
The cast is not a problem with rand struct, we are iterating through a
2 dimensional array and it does not matter in which order we iterate
the first dimension.
diff --git a/security/security.c b/security/security.c
index 2ee880b3a39a..4cc0e368d07f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -899,23 +899,24 @@ int lsm_fill_user_ctx(struct lsm_ctx __user
*uctx, u32 *uctx_len,
*/
int security_toggle_hook(void *hook_addr, bool state)
{
- struct lsm_static_call *scalls = ((void *)&static_calls_table);
+ struct lsm_static_call *scall;
+ void *scalls_table = ((void *)&static_calls_table);
unsigned long num_entries =
(sizeof(static_calls_table) / sizeof(struct lsm_static_call));
int i;
for (i = 0; i < num_entries; i++) {
-
- if (!scalls[i].hl || !scalls[i].hl->runtime)
+ scall = scalls_table + (i * sizeof(struct lsm_static_call));
+ if (!scall->hl || !scall->hl->runtime)
continue;
- if (scalls[i].hl->hook.lsm_func_addr != hook_addr)
+ if (scall->hl->hook.lsm_func_addr != hook_addr)
continue;
if (state)
- static_branch_enable(scalls[i].active);
+ static_branch_enable(scall->active);
else
- static_branch_disable(scalls[i].active);
+ static_branch_disable(scall->active);
return 0;
}
return -EINVAL;
fixes the error. I will respin.
> Either way, resolve this and make sure you test with GCC/randstruct
> enabled.
>
> > + unsigned long num_entries =
> > + (sizeof(static_calls_table) / sizeof(struct lsm_static_call));
> > + int i;
> > +
> > + for (i = 0; i < num_entries; i++) {
> > +
> > + if (!scalls[i].hl || !scalls[i].hl->runtime)
> > + continue;
> > +
> > + if (scalls[i].hl->hook.lsm_func_addr != hook_addr)
> > + continue;
> > +
> > + if (state)
> > + static_branch_enable(scalls[i].active);
> > + else
> > + static_branch_disable(scalls[i].active);
> > + return 0;
> > + }
> > + return -EINVAL;
> > +}
> > +
> > /*
> > * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
> > * can be accessed with:
> > --
> > 2.45.0.rc1.225.g2a3ae87e7f-goog
>
> --
> paul-moore.com
