On Sunday, November 01, 2015 06:24:32 PM Andreas Gruenbacher wrote:
> When fetching an inode's security label, check if it is still valid, and
> try reloading it if it is not. Reloading will fail when we are in RCU
> context which doesn't allow sleeping, or when we can't find a dentry for
> the inode. (Reloading happens via iop->getxattr which takes a dentry
> parameter.) When reloading fails, continue using the old, invalid
> label.
>
> Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>
> Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
Generally I would say that you made enough changes between v4 and v5 that
Stephen's ACK should have been dropped, but considering that he suggested the
changes I think's it's okay to leave it as-is.
Stephen, I'm reviewing/merging these changes now for the selinux#next-queue,
speak up if you have want your ACK removed.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index dcac6dc..0f94c2d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -241,11 +241,63 @@ static int inode_alloc_security(struct inode *inode)
> return 0;
> }
>
> +static int inode_doinit_with_dentry(struct inode *inode, struct dentry
> *opt_dentry); +
> +/*
> + * Try reloading inode security labels that have been marked as invalid.
> The + * @may_sleep parameter indicates when sleeping and thus reloading
> labels is + * allowed; when set to false, returns ERR_PTR(-ECHILD) when the
> label is + * invalid. The @opt_dentry parameter should be set to a dentry
> of the inode; + * when no dentry is available, set it to NULL instead.
> + */
> +static int __inode_security_revalidate(struct inode *inode,
> + struct dentry *opt_dentry,
> + bool may_sleep)
> +{
> + struct inode_security_struct *isec = inode->i_security;
> +
> + might_sleep_if(may_sleep);
> +
> + if (isec->initialized == LABEL_INVALID) {
> + if (!may_sleep)
> + return -ECHILD;
> +
> + /*
> + * Try reloading the inode security label. This will fail if
> + * @opt_dentry is NULL and no dentry for this inode can be
> + * found; in that case, continue using the old label.
> + */
> + inode_doinit_with_dentry(inode, opt_dentry);
> + }
> + return 0;
> +}
> +
> +static void inode_security_revalidate(struct inode *inode)
> +{
> + __inode_security_revalidate(inode, NULL, true);
> +}
> +
> +static struct inode_security_struct *inode_security_novalidate(struct inode
> *inode) +{
> + return inode->i_security;
> +}
> +
> +static struct inode_security_struct *inode_security_rcu(struct inode
> *inode, bool rcu) +{
> + int error;
> +
> + error = __inode_security_revalidate(inode, NULL, !rcu);
> + if (error)
> + return ERR_PTR(error);
> + return inode->i_security;
> +}
> +
> /*
> * Get the security label of an inode.
> */
> static struct inode_security_struct *inode_security(struct inode *inode)
> {
> + __inode_security_revalidate(inode, NULL, true);
> return inode->i_security;
> }
>
> @@ -256,6 +308,7 @@ static struct inode_security_struct
> *backing_inode_security(struct dentry *dentr {
> struct inode *inode = d_backing_inode(dentry);
>
> + __inode_security_revalidate(inode, dentry, true);
> return inode->i_security;
> }
>
> @@ -362,8 +415,6 @@ static const char *labeling_behaviors[7] = {
> "uses native labeling",
> };
>
> -static int inode_doinit_with_dentry(struct inode *inode, struct dentry
> *opt_dentry); -
> static inline int inode_doinit(struct inode *inode)
> {
> return inode_doinit_with_dentry(inode, NULL);
> @@ -1655,6 +1706,7 @@ static inline int dentry_has_perm(const struct cred
> *cred,
>
> ad.type = LSM_AUDIT_DATA_DENTRY;
> ad.u.dentry = dentry;
> + __inode_security_revalidate(inode, dentry, true);
> return inode_has_perm(cred, inode, av, &ad);
> }
>
> @@ -1670,6 +1722,7 @@ static inline int path_has_perm(const struct cred
> *cred,
>
> ad.type = LSM_AUDIT_DATA_PATH;
> ad.u.path = *path;
> + __inode_security_revalidate(inode, path->dentry, true);
> return inode_has_perm(cred, inode, av, &ad);
> }
>
> @@ -2874,7 +2927,9 @@ static int selinux_inode_follow_link(struct dentry
> *dentry, struct inode *inode, ad.type = LSM_AUDIT_DATA_DENTRY;
> ad.u.dentry = dentry;
> sid = cred_sid(cred);
> - isec = inode_security(inode);
> + isec = inode_security_rcu(inode, rcu);
> + if (IS_ERR(isec))
> + return PTR_ERR(isec);
>
> return avc_has_perm_flags(sid, isec->sid, isec->sclass, FILE__READ, &ad,
> rcu ? MAY_NOT_BLOCK : 0);
> @@ -2926,7 +2981,9 @@ static int selinux_inode_permission(struct inode
> *inode, int mask) perms = file_mask_to_av(inode->i_mode, mask);
>
> sid = cred_sid(cred);
> - isec = inode_security(inode);
> + isec = inode_security_rcu(inode, flags & MAY_NOT_BLOCK);
> + if (IS_ERR(isec))
> + return PTR_ERR(isec);
>
> rc = avc_has_perm_noaudit(sid, isec->sid, isec->sclass, perms, 0, &avd);
> audited = avc_audit_required(perms, &avd, rc,
> @@ -3234,6 +3291,7 @@ static int selinux_file_permission(struct file *file,
> int mask) /* No change since file_open check. */
> return 0;
>
> + inode_security_revalidate(inode);
> return selinux_revalidate_file_permission(file, mask);
> }
>
> @@ -3539,6 +3597,7 @@ static int selinux_file_open(struct file *file, const
> struct cred *cred) * new inode label or new policy.
> * This check is not redundant - do not remove.
> */
> + inode_security_revalidate(file_inode(file));
> return file_path_has_perm(cred, file, open_file_to_av(file));
> }
>
> @@ -4080,7 +4139,7 @@ static int selinux_socket_post_create(struct socket
> *sock, int family, int type, int protocol, int kern)
> {
> const struct task_security_struct *tsec = current_security();
> - struct inode_security_struct *isec = inode_security(SOCK_INODE(sock));
> + struct inode_security_struct *isec =
> inode_security_novalidate(SOCK_INODE(sock)); struct sk_security_struct
> *sksec;
> int err = 0;
>
> @@ -4280,9 +4339,9 @@ static int selinux_socket_accept(struct socket *sock,
> struct socket *newsock) if (err)
> return err;
>
> - newisec = inode_security(SOCK_INODE(newsock));
> + newisec = inode_security_novalidate(SOCK_INODE(newsock));
>
> - isec = inode_security(SOCK_INODE(sock));
> + isec = inode_security_novalidate(SOCK_INODE(sock));
> newisec->sclass = isec->sclass;
> newisec->sid = isec->sid;
> newisec->initialized = LABEL_INITIALIZED;
> @@ -4620,7 +4679,8 @@ static void selinux_sk_getsecid(struct sock *sk, u32
> *secid)
>
> static void selinux_sock_graft(struct sock *sk, struct socket *parent)
> {
> - struct inode_security_struct *isec = inode_security(SOCK_INODE(parent));
> + struct inode_security_struct *isec =
> + inode_security_novalidate(SOCK_INODE(parent));
> struct sk_security_struct *sksec = sk->sk_security;
>
> if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
--
paul moore
www.paul-moore.com
--
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
