Skip to content

PoC faster LSMs with static calls/keys: ask for feedbacks #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PaulRenauld wants to merge 5 commits into from
Closed

PoC faster LSMs with static calls/keys: ask for feedbacks #2

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e002a47
create keys and add to static list
PaulRenauld Jul 10, 2020
7a0e6b7
minimum working poc
PaulRenauld Jul 10, 2020
f990d59
wip
PaulRenauld Jul 13, 2020
ea2ba0c
cleanup
PaulRenauld Jul 13, 2020
71e7e3a
for-loop like
PaulRenauld Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 64 additions & 11 deletions security/security.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
#include <linux/string.h>
#include <linux/msg.h>
#include <net/flow.h>
#include <linux/static_call.h>
#include <linux/static_key.h>
#include <linux/printk.h>
#include <linux/bpf_lsm.h>


#define MAX_LSM_EVM_XATTR 2

Expand Down Expand Up @@ -464,6 +469,45 @@ static int lsm_append(const char *new, char **result)
return 0;
}

#define LSM_FUNC_DEFAULT(NAME) NAME##_func_default
#define HOOK_STATIC_CALL(HOOK, NUM) static_call_##HOOK##_##NUM
#define HOOK_STATIC_CHECK(HOOK, NUM) static_check_##HOOK##_##NUM

#define FOR_EACH_HOOK_SLOT(M, ...) \
M(1, __VA_ARGS__) \
M(2, __VA_ARGS__) \
M(3, __VA_ARGS__)

#define CREATE_STATIC(NUM, NAME) \
DEFINE_STATIC_CALL(HOOK_STATIC_CALL(NAME, NUM), LSM_FUNC_DEFAULT(NAME)); \
DEFINE_STATIC_KEY_FALSE(HOOK_STATIC_CHECK(NAME, NUM));

// We need a default function that will not be called so that
// static_call can infer the expected type
#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
noinline RET LSM_FUNC_DEFAULT(NAME)(__VA_ARGS__)\
{ \
return DEFAULT; \
} \
Comment on lines +488 to +491
Copy link
Copy Markdown
Owner Author

@PaulRenauld PaulRenauld Jul 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is defined but never used except for getting its type. We might be able to get rid of them by modifying the static calls.

Copy link
Copy Markdown
Collaborator

@bjackman bjackman Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah that's a tricky one. This also might present an intermediate solution: we only need one of these default functions for each possible value of (RET, DEFAULT), perhaps we can write some (potentially extremely galaxy-brain) macro hackery to avoid defining a different one for each hook.

FOR_EACH_HOOK_SLOT(CREATE_STATIC, NAME)
#include <linux/lsm_hook_defs.h>
#undef LSM_HOOK
#undef CREATE_STATIC

#define TRY_TO_ADD(NUM, HOOK, FUNC) \
if (!static_branch_unlikely(&HOOK_STATIC_CHECK(HOOK, NUM))) { \
static_call_update(HOOK_STATIC_CALL(HOOK, NUM), FUNC); \
static_branch_enable(&HOOK_STATIC_CHECK(HOOK, NUM)); \
break; \
}

#define add_static_hook(HOOK, FUNC) \
do { \
FOR_EACH_HOOK_SLOT(TRY_TO_ADD, HOOK, FUNC) \
printk(KERN_ERR "No slot remaining to add LSM hook for "\
#HOOK "\n"); \
} while(0)
Copy link
Copy Markdown
Collaborator

@bjackman bjackman Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's gonna be quite fiddly, but I think instead of having a globally fixed number of slots, and trying to put each hook into a slot one by one, we should try to:

  • determine the number of LSMs that exist at compile time, and set the number of slots to that-
  • at boot time, fix the mapping from LSMs to slot numbers.


/**
* security_add_hooks - Add a modules hooks to the hook lists.
* @hooks: the hooks to add
Expand All @@ -480,6 +524,12 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
for (i = 0; i < count; i++) {
hooks[i].lsm = lsm;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);

#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
if (&security_hook_heads.NAME == hooks[i].head) \
add_static_hook(NAME, hooks[i].hook.NAME);
#include <linux/lsm_hook_defs.h>
Comment on lines +529 to +531
Copy link
Copy Markdown
Owner Author

@PaulRenauld PaulRenauld Jul 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the ugliest part of this patch (and that's saying something).
Effctively, we will have ~200 ifs in the expanded code, and only one will be used. The difficulty here is that we need a generic solution, but the static calls/keys need to be referenced by their full name at compile time.
One possible solution would be to generate a function add_static_hook_##NAME(...) for each hook, and then modify security_hook_list and its creating macro LSM_HOOK_INIT to include a reference to the function. This would slow down slightly the initialization, and introduce ~200 new functions.

Copy link
Copy Markdown
Collaborator

@bjackman bjackman Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think another way to do this is to store the info needed for the static_call_update in the security_hook_list structure.

#undef LSM_HOOK
}

/*
Expand Down Expand Up @@ -687,6 +737,16 @@ static void __init lsm_early_task(struct task_struct *task)
#include <linux/lsm_hook_defs.h>
#undef LSM_HOOK


#define TRY_TO_STATIC_CALL_INT(NUM, R, HOOK, ...) \
if (static_branch_unlikely(&HOOK_STATIC_CHECK(HOOK, NUM))) { \
R = static_call(HOOK_STATIC_CALL(HOOK, NUM))(__VA_ARGS__); \
if (R != 0) \
break; \
}
Copy link
Copy Markdown
Collaborator

@bjackman bjackman Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were willing to get deep into the static_call implementation, I think it might be possible to avoid the static key. Let's discuss f2f.


#define TRY_TO_STATIC_CALL_VOID(NUM, HOOK, ...) \
static_call_cond(HOOK_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);
/*
* Hook list operation macros.
*
Expand All @@ -699,22 +759,15 @@ static void __init lsm_early_task(struct task_struct *task)

#define call_void_hook(FUNC, ...) \
do { \
struct security_hook_list *P; \
\
hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
P->hook.FUNC(__VA_ARGS__); \
FOR_EACH_HOOK_SLOT(TRY_TO_STATIC_CALL_VOID, \
FUNC, __VA_ARGS__) \
} while (0)

#define call_int_hook(FUNC, IRC, ...) ({ \
int RC = IRC; \
do { \
struct security_hook_list *P; \
\
hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
RC = P->hook.FUNC(__VA_ARGS__); \
if (RC != 0) \
break; \
} \
FOR_EACH_HOOK_SLOT(TRY_TO_STATIC_CALL_INT, \
RC, FUNC, __VA_ARGS__) \
} while (0); \
RC; \
})
Expand Down