On 5/22/2017 7:03 AM, Christoph Hellwig wrote: > On Sun, May 21, 2017 at 08:14:05PM +0900, Tetsuo Handa wrote: >> A sealable memory allocator patch was proposed at >> http://lkml.kernel.org/r/20170519103811.2183-1-igor.sto...@huawei.com , >> and is waiting for a follow-on patch showing how any of the kernel >> can be changed to use this new subsystem. So, here it is for LSM hooks. >> >> The LSM hooks ("struct security_hook_heads security_hook_heads" and >> "struct security_hook_list ...[]") will benefit from this allocator via >> protection using set_memory_ro()/set_memory_rw(), and it will remove >> CONFIG_SECURITY_WRITABLE_HOOKS config option. >> >> This means that these structures will be allocated at run time using >> smalloc(), and therefore the address of these structures will be >> determined at run time rather than compile time. >> >> But currently, LSM_HOOK_INIT() macro depends on the address of >> security_hook_heads being known at compile time. But we already >> initialize security_hook_heads as an array of "struct list_head". >> >> Therefore, let's use index number (or relative offset from the head >> of security_hook_heads) instead of absolute address of >> security_hook_heads so that LSM_HOOK_INIT() macro does not need to >> know absolute address of security_hook_heads. Then, security_add_hooks() >> will be able to allocate and copy "struct security_hook_list ...[]" using >> smalloc(). >> >> Signed-off-by: Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp> >> Cc: Kees Cook <keesc...@chromium.org> >> Cc: Paul Moore <p...@paul-moore.com> >> Cc: Stephen Smalley <s...@tycho.nsa.gov> >> Cc: Casey Schaufler <ca...@schaufler-ca.com> >> Cc: James Morris <james.l.mor...@oracle.com> >> Cc: Igor Stoppa <igor.sto...@huawei.com> >> Cc: Greg KH <gre...@linuxfoundation.org> >> --- >> include/linux/lsm_hooks.h | 6 +++--- >> security/security.c | 10 ++++++++-- >> 2 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h >> index 080f34e..865c11d 100644 >> --- a/include/linux/lsm_hooks.h >> +++ b/include/linux/lsm_hooks.h >> @@ -1884,8 +1884,8 @@ struct security_hook_heads { >> */ >> struct security_hook_list { >> struct list_head list; >> - struct list_head *head; >> union security_list_options hook; >> + const unsigned int idx; >> char *lsm; >> }; >> >> @@ -1896,9 +1896,9 @@ struct security_hook_list { >> * text involved. >> */ >> #define LSM_HOOK_INIT(HEAD, HOOK) \ >> - { .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } } >> + { .idx = offsetof(struct security_hook_heads, HEAD) / \ >> + sizeof(struct list_head), .hook = { .HEAD = HOOK } } >> >> -extern struct security_hook_heads security_hook_heads; >> extern char *lsm_names; >> >> extern void security_add_hooks(struct security_hook_list *hooks, int count, >> diff --git a/security/security.c b/security/security.c >> index 54b1e39..d6883ce 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -33,7 +33,7 @@ >> /* Maximum number of letters for an LSM name string */ >> #define SECURITY_NAME_MAX 10 >> >> -struct security_hook_heads security_hook_heads __lsm_ro_after_init; >> +static struct security_hook_heads security_hook_heads __lsm_ro_after_init; >> char *lsm_names; >> /* Boot-time LSM user choice */ >> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = >> @@ -152,10 +152,16 @@ void __init security_add_hooks(struct >> security_hook_list *hooks, int count, >> char *lsm) >> { >> int i; >> + struct list_head *list = (struct list_head *) &security_hook_heads; > Eww, struct casts. This whole security_hook_heads scheme stink, > even with the slight improvements from Tetsuo. It has everything we > shouldn't do - function pointers in structures that are not hard > read-only, structure casts, etc. > > What's the reason why can't just have good old const function tables?
The set of hooks used by most security modules are sparse. > Yeah, stackable LSM make that a little harder, but they should not be > enable by default anyway. With the number of security modules queued up behind full stacking I can't say that I agree with your assertion. > But even with those we can still chain > them together with a list with external linkage. I gave up that approach in 2012. Too many unnecessary calls to null functions, and massive function vectors with a tiny number of non-null entries. From a data structure standpoint, it was just wrong. The list scheme is exactly right for the task at hand. > -- > To unsubscribe from this list: send the line "unsubscribe > linux-security-module" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
