| Message ID | 157869192997.484726.14884768578207909170.stgit@chester (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RFC] selinux: remove redundant allocation and helper functions | expand |
On 1/10/2020 1:32 PM, Paul Moore wrote: > This patch removes the inode, file, and superblock security blob > allocation functions and moves the associated code into the > respective LSM hooks. This patch also removes the inode_doinit() > function as it was a trivial wrapper around > inode_doinit_with_dentry() and called from one location in the code. > > Signed-off-by: Paul Moore <paul@paul-moore.com> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> I don't usually comment on SELinux code, but I can't help but encourage this sort of clean-up. Thank you. > --- > security/selinux/hooks.c | 94 ++++++++++++++++++---------------------------- > 1 file changed, 36 insertions(+), 58 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 2c84b12d50bc..1305fc51bfae 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -238,24 +238,6 @@ static inline u32 task_sid(const struct task_struct *task) > return sid; > } > > -/* Allocate and free functions for each kind of security blob. */ > - > -static int inode_alloc_security(struct inode *inode) > -{ > - struct inode_security_struct *isec = selinux_inode(inode); > - u32 sid = current_sid(); > - > - spin_lock_init(&isec->lock); > - INIT_LIST_HEAD(&isec->list); > - isec->inode = inode; > - isec->sid = SECINITSID_UNLABELED; > - isec->sclass = SECCLASS_FILE; > - isec->task_sid = sid; > - isec->initialized = LABEL_INVALID; > - > - return 0; > -} > - > static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); > > /* > @@ -354,37 +336,6 @@ static void inode_free_security(struct inode *inode) > } > } > > -static int file_alloc_security(struct file *file) > -{ > - struct file_security_struct *fsec = selinux_file(file); > - u32 sid = current_sid(); > - > - fsec->sid = sid; > - fsec->fown_sid = sid; > - > - return 0; > -} > - > -static int superblock_alloc_security(struct super_block *sb) > -{ > - struct superblock_security_struct *sbsec; > - > - sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); > - if (!sbsec) > - return -ENOMEM; > - > - mutex_init(&sbsec->lock); > - INIT_LIST_HEAD(&sbsec->isec_head); > - spin_lock_init(&sbsec->isec_lock); > - sbsec->sb = sb; > - sbsec->sid = SECINITSID_UNLABELED; > - sbsec->def_sid = SECINITSID_FILE; > - sbsec->mntpoint_sid = SECINITSID_UNLABELED; > - sb->s_security = sbsec; > - > - return 0; > -} > - > static void superblock_free_security(struct super_block *sb) > { > struct superblock_security_struct *sbsec = sb->s_security; > @@ -406,11 +357,6 @@ static void selinux_free_mnt_opts(void *mnt_opts) > kfree(opts); > } > > -static inline int inode_doinit(struct inode *inode) > -{ > - return inode_doinit_with_dentry(inode, NULL); > -} > - > enum { > Opt_error = -1, > Opt_context = 0, > @@ -598,7 +544,7 @@ static int sb_finish_set_opts(struct super_block *sb) > inode = igrab(inode); > if (inode) { > if (!IS_PRIVATE(inode)) > - inode_doinit(inode); > + inode_doinit_with_dentry(inode, NULL); > iput(inode); > } > spin_lock(&sbsec->isec_lock); > @@ -2593,7 +2539,22 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) > > static int selinux_sb_alloc_security(struct super_block *sb) > { > - return superblock_alloc_security(sb); > + struct superblock_security_struct *sbsec; > + > + sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); > + if (!sbsec) > + return -ENOMEM; > + > + mutex_init(&sbsec->lock); > + INIT_LIST_HEAD(&sbsec->isec_head); > + spin_lock_init(&sbsec->isec_lock); > + sbsec->sb = sb; > + sbsec->sid = SECINITSID_UNLABELED; > + sbsec->def_sid = SECINITSID_FILE; > + sbsec->mntpoint_sid = SECINITSID_UNLABELED; > + sb->s_security = sbsec; > + > + return 0; > } > > static void selinux_sb_free_security(struct super_block *sb) > @@ -2845,7 +2806,18 @@ static int selinux_fs_context_parse_param(struct fs_context *fc, > > static int selinux_inode_alloc_security(struct inode *inode) > { > - return inode_alloc_security(inode); > + struct inode_security_struct *isec = selinux_inode(inode); > + u32 sid = current_sid(); > + > + spin_lock_init(&isec->lock); > + INIT_LIST_HEAD(&isec->list); > + isec->inode = inode; > + isec->sid = SECINITSID_UNLABELED; > + isec->sclass = SECCLASS_FILE; > + isec->task_sid = sid; > + isec->initialized = LABEL_INVALID; > + > + return 0; > } > > static void selinux_inode_free_security(struct inode *inode) > @@ -3555,7 +3527,13 @@ static int selinux_file_permission(struct file *file, int mask) > > static int selinux_file_alloc_security(struct file *file) > { > - return file_alloc_security(file); > + struct file_security_struct *fsec = selinux_file(file); > + u32 sid = current_sid(); > + > + fsec->sid = sid; > + fsec->fown_sid = sid; > + > + return 0; > } > > /* > >
On 1/10/20 4:32 PM, Paul Moore wrote: > This patch removes the inode, file, and superblock security blob > allocation functions and moves the associated code into the > respective LSM hooks. This patch also removes the inode_doinit() > function as it was a trivial wrapper around > inode_doinit_with_dentry() and called from one location in the code. > > Signed-off-by: Paul Moore <paul@paul-moore.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > security/selinux/hooks.c | 94 ++++++++++++++++++---------------------------- > 1 file changed, 36 insertions(+), 58 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 2c84b12d50bc..1305fc51bfae 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -238,24 +238,6 @@ static inline u32 task_sid(const struct task_struct *task) > return sid; > } > > -/* Allocate and free functions for each kind of security blob. */ > - > -static int inode_alloc_security(struct inode *inode) > -{ > - struct inode_security_struct *isec = selinux_inode(inode); > - u32 sid = current_sid(); > - > - spin_lock_init(&isec->lock); > - INIT_LIST_HEAD(&isec->list); > - isec->inode = inode; > - isec->sid = SECINITSID_UNLABELED; > - isec->sclass = SECCLASS_FILE; > - isec->task_sid = sid; > - isec->initialized = LABEL_INVALID; > - > - return 0; > -} > - > static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); > > /* > @@ -354,37 +336,6 @@ static void inode_free_security(struct inode *inode) > } > } > > -static int file_alloc_security(struct file *file) > -{ > - struct file_security_struct *fsec = selinux_file(file); > - u32 sid = current_sid(); > - > - fsec->sid = sid; > - fsec->fown_sid = sid; > - > - return 0; > -} > - > -static int superblock_alloc_security(struct super_block *sb) > -{ > - struct superblock_security_struct *sbsec; > - > - sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); > - if (!sbsec) > - return -ENOMEM; > - > - mutex_init(&sbsec->lock); > - INIT_LIST_HEAD(&sbsec->isec_head); > - spin_lock_init(&sbsec->isec_lock); > - sbsec->sb = sb; > - sbsec->sid = SECINITSID_UNLABELED; > - sbsec->def_sid = SECINITSID_FILE; > - sbsec->mntpoint_sid = SECINITSID_UNLABELED; > - sb->s_security = sbsec; > - > - return 0; > -} > - > static void superblock_free_security(struct super_block *sb) > { > struct superblock_security_struct *sbsec = sb->s_security; > @@ -406,11 +357,6 @@ static void selinux_free_mnt_opts(void *mnt_opts) > kfree(opts); > } > > -static inline int inode_doinit(struct inode *inode) > -{ > - return inode_doinit_with_dentry(inode, NULL); > -} > - > enum { > Opt_error = -1, > Opt_context = 0, > @@ -598,7 +544,7 @@ static int sb_finish_set_opts(struct super_block *sb) > inode = igrab(inode); > if (inode) { > if (!IS_PRIVATE(inode)) > - inode_doinit(inode); > + inode_doinit_with_dentry(inode, NULL); > iput(inode); > } > spin_lock(&sbsec->isec_lock); > @@ -2593,7 +2539,22 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) > > static int selinux_sb_alloc_security(struct super_block *sb) > { > - return superblock_alloc_security(sb); > + struct superblock_security_struct *sbsec; > + > + sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); > + if (!sbsec) > + return -ENOMEM; > + > + mutex_init(&sbsec->lock); > + INIT_LIST_HEAD(&sbsec->isec_head); > + spin_lock_init(&sbsec->isec_lock); > + sbsec->sb = sb; > + sbsec->sid = SECINITSID_UNLABELED; > + sbsec->def_sid = SECINITSID_FILE; > + sbsec->mntpoint_sid = SECINITSID_UNLABELED; > + sb->s_security = sbsec; > + > + return 0; > } > > static void selinux_sb_free_security(struct super_block *sb) > @@ -2845,7 +2806,18 @@ static int selinux_fs_context_parse_param(struct fs_context *fc, > > static int selinux_inode_alloc_security(struct inode *inode) > { > - return inode_alloc_security(inode); > + struct inode_security_struct *isec = selinux_inode(inode); > + u32 sid = current_sid(); > + > + spin_lock_init(&isec->lock); > + INIT_LIST_HEAD(&isec->list); > + isec->inode = inode; > + isec->sid = SECINITSID_UNLABELED; > + isec->sclass = SECCLASS_FILE; > + isec->task_sid = sid; > + isec->initialized = LABEL_INVALID; > + > + return 0; > } > > static void selinux_inode_free_security(struct inode *inode) > @@ -3555,7 +3527,13 @@ static int selinux_file_permission(struct file *file, int mask) > > static int selinux_file_alloc_security(struct file *file) > { > - return file_alloc_security(file); > + struct file_security_struct *fsec = selinux_file(file); > + u32 sid = current_sid(); > + > + fsec->sid = sid; > + fsec->fown_sid = sid; > + > + return 0; > } > > /* >
On Fri, Jan 10, 2020 at 4:32 PM Paul Moore <paul@paul-moore.com> wrote: > > This patch removes the inode, file, and superblock security blob > allocation functions and moves the associated code into the > respective LSM hooks. This patch also removes the inode_doinit() > function as it was a trivial wrapper around > inode_doinit_with_dentry() and called from one location in the code. > > Signed-off-by: Paul Moore <paul@paul-moore.com> > --- > security/selinux/hooks.c | 94 ++++++++++++++++++---------------------------- > 1 file changed, 36 insertions(+), 58 deletions(-) Merged into selinux/next.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2c84b12d50bc..1305fc51bfae 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -238,24 +238,6 @@ static inline u32 task_sid(const struct task_struct *task) return sid; } -/* Allocate and free functions for each kind of security blob. */ - -static int inode_alloc_security(struct inode *inode) -{ - struct inode_security_struct *isec = selinux_inode(inode); - u32 sid = current_sid(); - - spin_lock_init(&isec->lock); - INIT_LIST_HEAD(&isec->list); - isec->inode = inode; - isec->sid = SECINITSID_UNLABELED; - isec->sclass = SECCLASS_FILE; - isec->task_sid = sid; - isec->initialized = LABEL_INVALID; - - return 0; -} - static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); /* @@ -354,37 +336,6 @@ static void inode_free_security(struct inode *inode) } } -static int file_alloc_security(struct file *file) -{ - struct file_security_struct *fsec = selinux_file(file); - u32 sid = current_sid(); - - fsec->sid = sid; - fsec->fown_sid = sid; - - return 0; -} - -static int superblock_alloc_security(struct super_block *sb) -{ - struct superblock_security_struct *sbsec; - - sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); - if (!sbsec) - return -ENOMEM; - - mutex_init(&sbsec->lock); - INIT_LIST_HEAD(&sbsec->isec_head); - spin_lock_init(&sbsec->isec_lock); - sbsec->sb = sb; - sbsec->sid = SECINITSID_UNLABELED; - sbsec->def_sid = SECINITSID_FILE; - sbsec->mntpoint_sid = SECINITSID_UNLABELED; - sb->s_security = sbsec; - - return 0; -} - static void superblock_free_security(struct super_block *sb) { struct superblock_security_struct *sbsec = sb->s_security; @@ -406,11 +357,6 @@ static void selinux_free_mnt_opts(void *mnt_opts) kfree(opts); } -static inline int inode_doinit(struct inode *inode) -{ - return inode_doinit_with_dentry(inode, NULL); -} - enum { Opt_error = -1, Opt_context = 0, @@ -598,7 +544,7 @@ static int sb_finish_set_opts(struct super_block *sb) inode = igrab(inode); if (inode) { if (!IS_PRIVATE(inode)) - inode_doinit(inode); + inode_doinit_with_dentry(inode, NULL); iput(inode); } spin_lock(&sbsec->isec_lock); @@ -2593,7 +2539,22 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) static int selinux_sb_alloc_security(struct super_block *sb) { - return superblock_alloc_security(sb); + struct superblock_security_struct *sbsec; + + sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); + if (!sbsec) + return -ENOMEM; + + mutex_init(&sbsec->lock); + INIT_LIST_HEAD(&sbsec->isec_head); + spin_lock_init(&sbsec->isec_lock); + sbsec->sb = sb; + sbsec->sid = SECINITSID_UNLABELED; + sbsec->def_sid = SECINITSID_FILE; + sbsec->mntpoint_sid = SECINITSID_UNLABELED; + sb->s_security = sbsec; + + return 0; } static void selinux_sb_free_security(struct super_block *sb) @@ -2845,7 +2806,18 @@ static int selinux_fs_context_parse_param(struct fs_context *fc, static int selinux_inode_alloc_security(struct inode *inode) { - return inode_alloc_security(inode); + struct inode_security_struct *isec = selinux_inode(inode); + u32 sid = current_sid(); + + spin_lock_init(&isec->lock); + INIT_LIST_HEAD(&isec->list); + isec->inode = inode; + isec->sid = SECINITSID_UNLABELED; + isec->sclass = SECCLASS_FILE; + isec->task_sid = sid; + isec->initialized = LABEL_INVALID; + + return 0; } static void selinux_inode_free_security(struct inode *inode) @@ -3555,7 +3527,13 @@ static int selinux_file_permission(struct file *file, int mask) static int selinux_file_alloc_security(struct file *file) { - return file_alloc_security(file); + struct file_security_struct *fsec = selinux_file(file); + u32 sid = current_sid(); + + fsec->sid = sid; + fsec->fown_sid = sid; + + return 0; } /*
This patch removes the inode, file, and superblock security blob allocation functions and moves the associated code into the respective LSM hooks. This patch also removes the inode_doinit() function as it was a trivial wrapper around inode_doinit_with_dentry() and called from one location in the code. Signed-off-by: Paul Moore <paul@paul-moore.com> --- security/selinux/hooks.c | 94 ++++++++++++++++++---------------------------- 1 file changed, 36 insertions(+), 58 deletions(-)