| Message ID | 20180226203747.GB6886@avx2 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 26, 2018 at 3:37 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > Kmem caches are never reallocated once set up. > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > --- > > security/integrity/iint.c | 3 ++- > security/selinux/avc.c | 9 +++++---- > security/selinux/hooks.c | 5 +++-- > security/selinux/ss/avtab.c | 5 +++-- > security/selinux/ss/ebitmap.c | 3 ++- > security/selinux/ss/hashtab.c | 3 ++- > security/smack/smack_lsm.c | 3 ++- > 7 files changed, 19 insertions(+), 12 deletions(-) In the future you're probably better off separating the SELinux, Smack, and IMA pieces into separate patches. SELinux comments inline ... > --- a/security/selinux/ss/avtab.c > +++ b/security/selinux/ss/avtab.c > @@ -17,14 +17,15 @@ > * Tuned number of hash slots for avtab to reduce memory usage > */ > > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > #include "avtab.h" > #include "policydb.h" > > -static struct kmem_cache *avtab_node_cachep; > -static struct kmem_cache *avtab_xperms_cachep; > +static struct kmem_cache *avtab_node_cachep __ro_after_init; > +static struct kmem_cache *avtab_xperms_cachep __ro_after_init; Both avtab_node_cachep and avtab_xperms_cachep get allocated in avtab_cache_init() which is called during the first policy load (from userspace) which happens after we are past __init, yes? This is why we don't mark avtab_cache_init() with the __init macro. > --- a/security/selinux/ss/ebitmap.c > +++ b/security/selinux/ss/ebitmap.c > @@ -16,6 +16,7 @@ > * Applied standard bit operations to improve bitmap scanning. > */ > > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > @@ -25,7 +26,7 @@ > > #define BITS_PER_U64 (sizeof(u64) * 8) > > -static struct kmem_cache *ebitmap_node_cachep; > +static struct kmem_cache *ebitmap_node_cachep __ro_after_init; Same. > --- a/security/selinux/ss/hashtab.c > +++ b/security/selinux/ss/hashtab.c > @@ -4,13 +4,14 @@ > * > * Author : Stephen Smalley, <sds@tycho.nsa.gov> > */ > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > #include <linux/sched.h> > #include "hashtab.h" > > -static struct kmem_cache *hashtab_node_cachep; > +static struct kmem_cache *hashtab_node_cachep __ro_after_init; Same.
On 2/26/2018 12:37 PM, Alexey Dobriyan wrote: > Kmem caches are never reallocated once set up. > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > --- > > security/integrity/iint.c | 3 ++- > security/selinux/avc.c | 9 +++++---- > security/selinux/hooks.c | 5 +++-- > security/selinux/ss/avtab.c | 5 +++-- > security/selinux/ss/ebitmap.c | 3 ++- > security/selinux/ss/hashtab.c | 3 ++- > security/smack/smack_lsm.c | 3 ++- > 7 files changed, 19 insertions(+), 12 deletions(-) > > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -15,6 +15,7 @@ > * - cache integrity information associated with an inode > * using a rbtree tree. > */ > +#include <linux/cache.h> > #include <linux/slab.h> > #include <linux/module.h> > #include <linux/spinlock.h> > @@ -25,7 +26,7 @@ > > static struct rb_root integrity_iint_tree = RB_ROOT; > static DEFINE_RWLOCK(integrity_iint_lock); > -static struct kmem_cache *iint_cache __read_mostly; > +static struct kmem_cache *iint_cache __ro_after_init; > > /* > * __integrity_iint_find - return the iint associated with an inode > --- a/security/selinux/avc.c > +++ b/security/selinux/avc.c > @@ -13,6 +13,7 @@ > * it under the terms of the GNU General Public License version 2, > * as published by the Free Software Foundation. > */ > +#include <linux/cache.h> > #include <linux/types.h> > #include <linux/stddef.h> > #include <linux/kernel.h> > @@ -91,10 +92,10 @@ DEFINE_PER_CPU(struct avc_cache_stats, avc_cache_stats) = { 0 }; > > static struct avc_cache avc_cache; > static struct avc_callback_node *avc_callbacks; > -static struct kmem_cache *avc_node_cachep; > -static struct kmem_cache *avc_xperms_data_cachep; > -static struct kmem_cache *avc_xperms_decision_cachep; > -static struct kmem_cache *avc_xperms_cachep; > +static struct kmem_cache *avc_node_cachep __ro_after_init; > +static struct kmem_cache *avc_xperms_data_cachep __ro_after_init; > +static struct kmem_cache *avc_xperms_decision_cachep __ro_after_init; > +static struct kmem_cache *avc_xperms_cachep __ro_after_init; > > static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass) > { > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -24,6 +24,7 @@ > * as published by the Free Software Foundation. > */ > > +#include <linux/cache.h> > #include <linux/init.h> > #include <linux/kd.h> > #include <linux/kernel.h> > @@ -129,8 +130,8 @@ __setup("selinux=", selinux_enabled_setup); > int selinux_enabled = 1; > #endif > > -static struct kmem_cache *sel_inode_cache; > -static struct kmem_cache *file_security_cache; > +static struct kmem_cache *sel_inode_cache __ro_after_init; > +static struct kmem_cache *file_security_cache __ro_after_init; > > /** > * selinux_secmark_enabled - Check to see if SECMARK is currently enabled > --- a/security/selinux/ss/avtab.c > +++ b/security/selinux/ss/avtab.c > @@ -17,14 +17,15 @@ > * Tuned number of hash slots for avtab to reduce memory usage > */ > > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > #include "avtab.h" > #include "policydb.h" > > -static struct kmem_cache *avtab_node_cachep; > -static struct kmem_cache *avtab_xperms_cachep; > +static struct kmem_cache *avtab_node_cachep __ro_after_init; > +static struct kmem_cache *avtab_xperms_cachep __ro_after_init; > > /* Based on MurmurHash3, written by Austin Appleby and placed in the > * public domain. > --- a/security/selinux/ss/ebitmap.c > +++ b/security/selinux/ss/ebitmap.c > @@ -16,6 +16,7 @@ > * Applied standard bit operations to improve bitmap scanning. > */ > > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > @@ -25,7 +26,7 @@ > > #define BITS_PER_U64 (sizeof(u64) * 8) > > -static struct kmem_cache *ebitmap_node_cachep; > +static struct kmem_cache *ebitmap_node_cachep __ro_after_init; > > int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2) > { > --- a/security/selinux/ss/hashtab.c > +++ b/security/selinux/ss/hashtab.c > @@ -4,13 +4,14 @@ > * > * Author : Stephen Smalley, <sds@tycho.nsa.gov> > */ > +#include <linux/cache.h> > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/errno.h> > #include <linux/sched.h> > #include "hashtab.h" > > -static struct kmem_cache *hashtab_node_cachep; > +static struct kmem_cache *hashtab_node_cachep __ro_after_init; > > struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *key), > int (*keycmp)(struct hashtab *h, const void *key1, const void *key2), > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -18,6 +18,7 @@ > * as published by the Free Software Foundation. > */ > > +#include <linux/cache.h> > #include <linux/xattr.h> > #include <linux/pagemap.h> > #include <linux/mount.h> > @@ -55,7 +56,7 @@ > DEFINE_MUTEX(smack_ipv6_lock); > static LIST_HEAD(smk_ipv6_port_list); > #endif > -static struct kmem_cache *smack_inode_cache; > +static struct kmem_cache *smack_inode_cache __ro_after_init; Have you tried this to see if it works? > int smack_enabled; > > static const match_table_t smk_mount_tokens = { > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/27/2018 05:19 PM, Paul Moore wrote: > On Mon, Feb 26, 2018 at 3:37 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: >> Kmem caches are never reallocated once set up. >> >> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> >> --- >> >> security/integrity/iint.c | 3 ++- >> security/selinux/avc.c | 9 +++++---- >> security/selinux/hooks.c | 5 +++-- >> security/selinux/ss/avtab.c | 5 +++-- >> security/selinux/ss/ebitmap.c | 3 ++- >> security/selinux/ss/hashtab.c | 3 ++- >> security/smack/smack_lsm.c | 3 ++- >> 7 files changed, 19 insertions(+), 12 deletions(-) > > In the future you're probably better off separating the SELinux, > Smack, and IMA pieces into separate patches. SELinux comments inline > ... > >> --- a/security/selinux/ss/avtab.c >> +++ b/security/selinux/ss/avtab.c >> @@ -17,14 +17,15 @@ >> * Tuned number of hash slots for avtab to reduce memory usage >> */ >> >> +#include <linux/cache.h> >> #include <linux/kernel.h> >> #include <linux/slab.h> >> #include <linux/errno.h> >> #include "avtab.h" >> #include "policydb.h" >> >> -static struct kmem_cache *avtab_node_cachep; >> -static struct kmem_cache *avtab_xperms_cachep; >> +static struct kmem_cache *avtab_node_cachep __ro_after_init; >> +static struct kmem_cache *avtab_xperms_cachep __ro_after_init; > > Both avtab_node_cachep and avtab_xperms_cachep get allocated in > avtab_cache_init() which is called during the first policy load (from > userspace) which happens after we are past __init, yes? > > This is why we don't mark avtab_cache_init() with the __init macro. NB My "selinux: wrap global selinux state" patch moves this initialization to selinux_init(), at which point we can in fact mark these caches this way. I think that is more correct anyway, but it was specifically motivated by the fact that we only want to perform this initialization once and first policy load becomes a per-state/namespace operation. > >> --- a/security/selinux/ss/ebitmap.c >> +++ b/security/selinux/ss/ebitmap.c >> @@ -16,6 +16,7 @@ >> * Applied standard bit operations to improve bitmap scanning. >> */ >> >> +#include <linux/cache.h> >> #include <linux/kernel.h> >> #include <linux/slab.h> >> #include <linux/errno.h> >> @@ -25,7 +26,7 @@ >> >> #define BITS_PER_U64 (sizeof(u64) * 8) >> >> -static struct kmem_cache *ebitmap_node_cachep; >> +static struct kmem_cache *ebitmap_node_cachep __ro_after_init; > > Same. > >> --- a/security/selinux/ss/hashtab.c >> +++ b/security/selinux/ss/hashtab.c >> @@ -4,13 +4,14 @@ >> * >> * Author : Stephen Smalley, <sds@tycho.nsa.gov> >> */ >> +#include <linux/cache.h> >> #include <linux/kernel.h> >> #include <linux/slab.h> >> #include <linux/errno.h> >> #include <linux/sched.h> >> #include "hashtab.h" >> >> -static struct kmem_cache *hashtab_node_cachep; >> +static struct kmem_cache *hashtab_node_cachep __ro_after_init; > > Same. > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 28, 2018 at 09:34:07AM -0500, Stephen Smalley wrote: > On 02/27/2018 05:19 PM, Paul Moore wrote: > > This is why we don't mark avtab_cache_init() with the __init macro. > > NB My "selinux: wrap global selinux state" patch moves this > initialization to selinux_init(), at which point we can in fact mark > these caches this way. I think that is more correct anyway, but it was > specifically motivated by the fact that we only want to perform this > initialization once and first policy load becomes a per-state/namespace > operation. I got confused by SLAB_PANIC markings. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -15,6 +15,7 @@ * - cache integrity information associated with an inode * using a rbtree tree. */ +#include <linux/cache.h> #include <linux/slab.h> #include <linux/module.h> #include <linux/spinlock.h> @@ -25,7 +26,7 @@ static struct rb_root integrity_iint_tree = RB_ROOT; static DEFINE_RWLOCK(integrity_iint_lock); -static struct kmem_cache *iint_cache __read_mostly; +static struct kmem_cache *iint_cache __ro_after_init; /* * __integrity_iint_find - return the iint associated with an inode --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -13,6 +13,7 @@ * it under the terms of the GNU General Public License version 2, * as published by the Free Software Foundation. */ +#include <linux/cache.h> #include <linux/types.h> #include <linux/stddef.h> #include <linux/kernel.h> @@ -91,10 +92,10 @@ DEFINE_PER_CPU(struct avc_cache_stats, avc_cache_stats) = { 0 }; static struct avc_cache avc_cache; static struct avc_callback_node *avc_callbacks; -static struct kmem_cache *avc_node_cachep; -static struct kmem_cache *avc_xperms_data_cachep; -static struct kmem_cache *avc_xperms_decision_cachep; -static struct kmem_cache *avc_xperms_cachep; +static struct kmem_cache *avc_node_cachep __ro_after_init; +static struct kmem_cache *avc_xperms_data_cachep __ro_after_init; +static struct kmem_cache *avc_xperms_decision_cachep __ro_after_init; +static struct kmem_cache *avc_xperms_cachep __ro_after_init; static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass) { --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -24,6 +24,7 @@ * as published by the Free Software Foundation. */ +#include <linux/cache.h> #include <linux/init.h> #include <linux/kd.h> #include <linux/kernel.h> @@ -129,8 +130,8 @@ __setup("selinux=", selinux_enabled_setup); int selinux_enabled = 1; #endif -static struct kmem_cache *sel_inode_cache; -static struct kmem_cache *file_security_cache; +static struct kmem_cache *sel_inode_cache __ro_after_init; +static struct kmem_cache *file_security_cache __ro_after_init; /** * selinux_secmark_enabled - Check to see if SECMARK is currently enabled --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -17,14 +17,15 @@ * Tuned number of hash slots for avtab to reduce memory usage */ +#include <linux/cache.h> #include <linux/kernel.h> #include <linux/slab.h> #include <linux/errno.h> #include "avtab.h" #include "policydb.h" -static struct kmem_cache *avtab_node_cachep; -static struct kmem_cache *avtab_xperms_cachep; +static struct kmem_cache *avtab_node_cachep __ro_after_init; +static struct kmem_cache *avtab_xperms_cachep __ro_after_init; /* Based on MurmurHash3, written by Austin Appleby and placed in the * public domain. --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -16,6 +16,7 @@ * Applied standard bit operations to improve bitmap scanning. */ +#include <linux/cache.h> #include <linux/kernel.h> #include <linux/slab.h> #include <linux/errno.h> @@ -25,7 +26,7 @@ #define BITS_PER_U64 (sizeof(u64) * 8) -static struct kmem_cache *ebitmap_node_cachep; +static struct kmem_cache *ebitmap_node_cachep __ro_after_init; int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2) { --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -4,13 +4,14 @@ * * Author : Stephen Smalley, <sds@tycho.nsa.gov> */ +#include <linux/cache.h> #include <linux/kernel.h> #include <linux/slab.h> #include <linux/errno.h> #include <linux/sched.h> #include "hashtab.h" -static struct kmem_cache *hashtab_node_cachep; +static struct kmem_cache *hashtab_node_cachep __ro_after_init; struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *key), int (*keycmp)(struct hashtab *h, const void *key1, const void *key2), --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -18,6 +18,7 @@ * as published by the Free Software Foundation. */ +#include <linux/cache.h> #include <linux/xattr.h> #include <linux/pagemap.h> #include <linux/mount.h> @@ -55,7 +56,7 @@ DEFINE_MUTEX(smack_ipv6_lock); static LIST_HEAD(smk_ipv6_port_list); #endif -static struct kmem_cache *smack_inode_cache; +static struct kmem_cache *smack_inode_cache __ro_after_init; int smack_enabled; static const match_table_t smk_mount_tokens = {
Kmem caches are never reallocated once set up. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- security/integrity/iint.c | 3 ++- security/selinux/avc.c | 9 +++++---- security/selinux/hooks.c | 5 +++-- security/selinux/ss/avtab.c | 5 +++-- security/selinux/ss/ebitmap.c | 3 ++- security/selinux/ss/hashtab.c | 3 ++- security/smack/smack_lsm.c | 3 ++- 7 files changed, 19 insertions(+), 12 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html