* [PATCH] selinux: consilidate comments from inode_doinit_with_dentry wrt !dentry
@ 2021-10-11 19:02 Davidlohr Bueso
2021-10-11 23:13 ` Paul Moore
0 siblings, 1 reply; 2+ messages in thread
From: Davidlohr Bueso @ 2021-10-11 19:02 UTC (permalink / raw)
To: paul; +Cc: stephen.smalley.work, selinux, linux-kernel, dave, Davidlohr Bueso
Instead of having the same big comment twice for the same scenario,
just have it in out_invalid.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
security/selinux/hooks.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e7ebd45ca345..0e0013cc7ba8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1502,18 +1502,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
if (!dentry)
dentry = d_find_any_alias(inode);
}
- if (!dentry) {
- /*
- * this is 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_invalid;
- }
rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid,
&sid);
@@ -1559,15 +1549,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
if (!dentry)
dentry = d_find_any_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_invalid;
rc = selinux_genfs_get_sid(dentry, sclass,
@@ -1607,6 +1588,15 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
return rc;
out_invalid:
+ /*
+ * This is 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.
+ */
spin_lock(&isec->lock);
if (isec->initialized == LABEL_PENDING) {
isec->initialized = LABEL_INVALID;
--
2.26.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] selinux: consilidate comments from inode_doinit_with_dentry wrt !dentry
2021-10-11 19:02 [PATCH] selinux: consilidate comments from inode_doinit_with_dentry wrt !dentry Davidlohr Bueso
@ 2021-10-11 23:13 ` Paul Moore
0 siblings, 0 replies; 2+ messages in thread
From: Paul Moore @ 2021-10-11 23:13 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: Stephen Smalley, selinux, linux-kernel, Davidlohr Bueso
On Mon, Oct 11, 2021 at 3:02 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> Instead of having the same big comment twice for the same scenario,
> just have it in out_invalid.
>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
> security/selinux/hooks.c | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
Hi Davidlohr,
Thanks for the patch, but I think the comment is a bit more
discoverable/helpful when it is integrated into the main flow of the
code and not the error handling at the bottom. However, if you wanted
to change the second instance to refer to the prior instance I think
that would be okay.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e7ebd45ca345..0e0013cc7ba8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1502,18 +1502,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> if (!dentry)
> dentry = d_find_any_alias(inode);
> }
> - if (!dentry) {
> - /*
> - * this is 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_invalid;
> - }
>
> rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid,
> &sid);
> @@ -1559,15 +1549,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> if (!dentry)
> dentry = d_find_any_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_invalid;
> rc = selinux_genfs_get_sid(dentry, sclass,
> @@ -1607,6 +1588,15 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> return rc;
>
> out_invalid:
> + /*
> + * This is 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.
> + */
> spin_lock(&isec->lock);
> if (isec->initialized == LABEL_PENDING) {
> isec->initialized = LABEL_INVALID;
> --
> 2.26.2
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-10-11 23:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 19:02 [PATCH] selinux: consilidate comments from inode_doinit_with_dentry wrt !dentry Davidlohr Bueso
2021-10-11 23:13 ` Paul Moore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.