selinux: correctly label /proc inodes in use before the policy is loaded
PATCH 1/1 selinux: correctly label /proc inodes in use before the policy is loaded
This patch is based on an earlier patch by Eric Paris, he describes the problem below:
"If an inode is accessed before policy load it will get placed on a
list of inodes to be initialized after policy load. After policy load we call inode_doinit() which calls inode_doinit_with_dentry() on all inodes accessed before policy load. In the case of inodes in procfs that means we'll end up at the bottom where it does:
/* Default to the fs superblock SID. */
isec->sid = sbsec->sid;
if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) {
if (opt_dentry) {
isec->sclass = inode_mode_to_security_class(...)
rc = selinux_proc_get_sid(opt_dentry,
isec->sclass,
&sid);
if (rc)
goto out_unlock;
isec->sid = sid;
}
} Since opt_dentry is null, we'll never call selinux_proc_get_sid() and will leave the inode labeled with the label on the superblock. I believe a fix would be to mimic the behavior of xattrs. Look for an alias of the inode. If it can't be found, just leave the inode uninitialized (and pick it up later) if it can be found, we should be able to call selinux_proc_get_sid() ..."
On a system exhibiting this problem, you will notice a lot of files in /proc with the generic "proc_t" type (at least the ones that were accessed early in the boot), for example:
# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }'
system_u:object_r:proc_t:s0 /proc/sys/kernel/shmmaxHowever, with this patch in place we see the expected result:
# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }'
system_u:object_r:sysctl_kernel_t:s0 /proc/sys/kernel/shmmax--- security/selinux/hooks.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
security/selinux/hooks.c +26 -9
@@ -1419,15 +1419,32 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid;if ((sbsec->flags &SE_SBPROC ) && !S_ISLNK (inode->i_mode)) { -if (opt_dentry) { - isec->sclass =inode_mode_to_security_class (inode->i_mode); - rc =selinux_proc_get_sid (opt_dentry, - isec->sclass, - &sid); -if (rc) - goto out_unlock; - isec->sid = sid; - } +/* Need a dentry, since the procfs API requires one. */ +if (opt_dentry) +/* Called from d_instantiate or +* d_splice_alias. */ + dentry =dget (opt_dentry); +else +/* Called from selinux_complete_init, try to +* find a dentry. */ + dentry =d_find_alias (inode); +/* +* This can be hit on boot when a file is accessed +* before the policy is loaded. When we load policy we +* may find inodes that have no dentry on the +* sbsec->isec_head list. No reason to complain as +* these will get fixed up the next time we go through +* inode_doinit() with a dentry, before these inodes +* could be used again by userspace. +*/ +if (!dentry) + goto out_unlock; + isec->sclass =inode_mode_to_security_class (inode->i_mode); + rc =selinux_proc_get_sid (dentry, isec->sclass, &sid); +dput (dentry); +if (rc) + goto out_unlock; + isec->sid = sid; }break ; }
Quoted 57 lines · Paul Moore
This patch is based on an earlier patch by Eric Paris, he describes the problem below:
"If an inode is accessed before policy load it will get placed on a
list of inodes to be initialized after policy load. After policy load we callinode_doinit()which callsinode_doinit_with_dentry()on all inodes accessed before policy load. In the case of inodes in procfs that means we'll end up at the bottom where it does:/* Default to the fs superblock SID. */ isec->sid = sbsec->sid; if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { if (opt_dentry) { isec->sclass = inode_mode_to_security_class(...) rc = selinux_proc_get_sid(opt_dentry, isec->sclass, &sid); if (rc) goto out_unlock; isec->sid = sid; } }Since
opt_dentryis null, we'll never callselinux_proc_get_sid()and will leave the inode labeled with the label on the superblock. I believe a fix would be to mimic the behavior of xattrs. Look for an alias of the inode. If it can't be found, just leave the inode uninitialized (and pick it up later) if it can be found, we should be able to callselinux_proc_get_sid()..."On a system exhibiting this problem, you will notice a lot of files in /proc with the generic "
proc_t" type (at least the ones that were accessed early in the boot), for example:# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:proc_t:s0 /proc/sys/kernel/shmmaxHowever, with this patch in place we see the expected result:
# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:sysctl_kernel_t:s0 /proc/sys/kernel/shmmaxCc Eric Paris <[email protected]>Signed-off-by Paul Moore <[email protected]>--- security/selinux/hooks.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)security/selinux/hooks.c +0
-0@@ -1419,15 +1419,32 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid;if ((sbsec->flags &SE_SBPROC ) && !S_ISLNK (inode->i_mode)) {
security/selinux/hooks.c static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent · line 1422
Quoted 10 lines
security/selinux/hooks.c +1
-9-if (opt_dentry) { - isec->sclass =inode_mode_to_security_class (inode->i_mode); - rc =selinux_proc_get_sid (opt_dentry, - isec->sclass, - &sid); -if (rc) - goto out_unlock; - isec->sid = sid; - } +/* Need a dentry, since the procfs API requires one. */
Comment isn't accurate; unlike the xattr case where the dentry requirement originates from the ->getxattr API, here we need a dentry for our own internal selinux_proc_get_sid() helper. Otherwise, looks fine.
Quoted 36 lines
security/selinux/hooks.c +25
-0+if (opt_dentry) +/* Called from d_instantiate or +* d_splice_alias. */ + dentry =dget (opt_dentry); +else +/* Called from selinux_complete_init, try to +* find a dentry. */ + dentry =d_find_alias (inode); +/* +* This can be hit on boot when a file is accessed +* before the policy is loaded. When we load policy we +* may find inodes that have no dentry on the +* sbsec->isec_head list. No reason to complain as +* these will get fixed up the next time we go through +* inode_doinit() with a dentry, before these inodes +* could be used again by userspace. +*/ +if (!dentry) + goto out_unlock; + isec->sclass =inode_mode_to_security_class (inode->i_mode); + rc =selinux_proc_get_sid (dentry, isec->sclass, &sid); +dput (dentry); +if (rc) + goto out_unlock; + isec->sid = sid; }break ; }_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].
Eric Paris
Acked-byQuoted 57 lines · Stephen Smalley
Quoted 56 lines · Paul Moore
This patch is based on an earlier patch by Eric Paris, he describes the problem below:
"If an inode is accessed before policy load it will get placed on a
list of inodes to be initialized after policy load. After policy load we callinode_doinit()which callsinode_doinit_with_dentry()on all inodes accessed before policy load. In the case of inodes in procfs that means we'll end up at the bottom where it does:/* Default to the fs superblock SID. */ isec->sid = sbsec->sid; if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { if (opt_dentry) { isec->sclass = inode_mode_to_security_class(...) rc = selinux_proc_get_sid(opt_dentry, isec->sclass, &sid); if (rc) goto out_unlock; isec->sid = sid; } }Since
opt_dentryis null, we'll never callselinux_proc_get_sid()and will leave the inode labeled with the label on the superblock. I believe a fix would be to mimic the behavior of xattrs. Look for an alias of the inode. If it can't be found, just leave the inode uninitialized (and pick it up later) if it can be found, we should be able to callselinux_proc_get_sid()..."On a system exhibiting this problem, you will notice a lot of files in /proc with the generic "
proc_t" type (at least the ones that were accessed early in the boot), for example:# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:proc_t:s0 /proc/sys/kernel/shmmaxHowever, with this patch in place we see the expected result:
# ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:sysctl_kernel_t:s0 /proc/sys/kernel/shmmaxCc Eric Paris <[email protected]>Signed-off-by Paul Moore <[email protected]>--- security/selinux/hooks.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)security/selinux/hooks.c +0
-0@@ -1419,15 +1419,32 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid;
Quoted 15 lines
security/selinux/hooks.c static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent · line 1422
Quoted 11 lines
security/selinux/hooks.c +1
-9if ((sbsec->flags &SE_SBPROC ) && !S_ISLNK (inode->i_mode)) { -if (opt_dentry) { - isec->sclass =inode_mode_to_security_class (inode->i_mode); - rc =selinux_proc_get_sid (opt_dentry, - isec->sclass, - &sid); -if (rc) - goto out_unlock; - isec->sid = sid; - } +/* Need a dentry, since the procfs API requires one. */ Comment isn't accurate; unlike the xattr case where the dentry requirement originates from the ->getxattr API, here we need a dentry for our own internal
selinux_proc_get_sid()helper. Otherwise, looks fine.
I guess I could have written that comment better...
/* We must have a dentry to determine the label on procfs inodes */
With a comment change like that
Quoted 39 lines
Quoted 37 lines
security/selinux/hooks.c +25
-0+if (opt_dentry) +/* Called from d_instantiate or +* d_splice_alias. */ + dentry =dget (opt_dentry); +else +/* Called from selinux_complete_init, try to +* find a dentry. */ + dentry =d_find_alias (inode); +/* +* This can be hit on boot when a file is accessed +* before the policy is loaded. When we load policy we +* may find inodes that have no dentry on the +* sbsec->isec_head list. No reason to complain as +* these will get fixed up the next time we go through +* inode_doinit() with a dentry, before these inodes +* could be used again by userspace. +*/ +if (!dentry) + goto out_unlock; + isec->sclass =inode_mode_to_security_class (inode->i_mode); + rc =selinux_proc_get_sid (dentry, isec->sclass, &sid); +dput (dentry); +if (rc) + goto out_unlock; + isec->sid = sid; }break ; }_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].
Quoted 29 lines · Eric Paris
Quoted 9 lines · Stephen Smalley
Quoted 8 lines · Paul Moore
security/selinux/hooks.c +0
-0@@ -1419,15 +1419,32 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent> > isec->sid = sbsec->sid;Quoted 4 lines
security/selinux/hooks.c static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
Quoted 4 lines
security/selinux/hooks.c +0
-2if ((sbsec->flags &SE_SBPROC ) && !S_ISLNK (inode->i_mode)) { -if (opt_dentry) { - isec->sclass = inode_mode_to_security_class(inode-
i_mode);Quoted 5 lines
Quoted 5 lines
security/selinux/hooks.c +0
-5- rc =selinux_proc_get_sid (opt_dentry, - isec->sclass, - &sid); -if (rc) - goto out_unlock;Quoted 8 lines
security/selinux/hooks.c static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent · line 1422
Quoted 3 lines
security/selinux/hooks.c +1
-2- isec->sid = sid; - } +/* Need a dentry, since the procfs API requires one. */ Comment isn't accurate; unlike the xattr case where the dentry requirement originates from the ->getxattr API, here we need a dentry for our own internal
selinux_proc_get_sid()helper. Otherwise, looks fine.
Quoted 7 lines
I guess I could have written that comment better...
/* We must have a dentry to determine the label on procfs inodes */
With a comment change like that
Acked-by Eric Paris <[email protected]>
Updated. I'll push it to next later today.
Signature · 2 lines
paul moore security and virtualization @ redhat