linux-kernel - Re: [RFC PATCH 1/2] selinux: Don't call avc_compute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5e51f9a5-ba76-a42d-fc2b-9255f8544859@tycho.nsa.gov>
Date:   Tue, 19 Nov 2019 13:59:40 -0500
From:   Stephen Smalley <sds@...ho.nsa.gov>
To:     Will Deacon <will@...nel.org>, selinux@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] selinux: Don't call avc_compute_av() from RCU
 path walk

On 11/19/19 1:40 PM, Will Deacon wrote:
> 'avc_compute_av()' can block, so we carefully exit the RCU read-side
> critical section before calling it in 'avc_has_perm_noaudit()'.
> Unfortunately, if we're calling from the VFS layer on the RCU path walk
> via 'selinux_inode_permission()' then we're still actually in an RCU
> read-side critical section and must not block.

avc_compute_av() should never block AFAIK. The blocking concern was with 
slow_avc_audit(), and even that appears dubious to me. That seems to be 
more about misuse of d_find_alias in dump_common_audit_data() than anything.

> 
> 'avc_denied()' already handles this by simply returning success and
> postponing the auditing until we're called again on the slowpath, so
> follow the same approach here and return early if the node lookup fails
> on the RCU walk path.
> 
> Signed-off-by: Will Deacon <will@...nel.org>
> ---
>   security/selinux/avc.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index ecd3829996aa..9c183c899e92 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -1159,16 +1159,19 @@ inline int avc_has_perm_noaudit(struct selinux_state *state,
>   	rcu_read_lock();
>   
>   	node = avc_lookup(state->avc, ssid, tsid, tclass);
> -	if (unlikely(!node))
> +	if (unlikely(!node)) {
> +		if (flags & AVC_NONBLOCKING)
> +			goto out;
>   		node = avc_compute_av(state, ssid, tsid, tclass, avd, &xp_node);
> -	else
> +	} else {
>   		memcpy(avd, &node->ae.avd, sizeof(*avd));
> +	}
>   
>   	denied = requested & ~(avd->allowed);
>   	if (unlikely(denied))
>   		rc = avc_denied(state, ssid, tsid, tclass, requested, 0, 0,
>   				flags, avd);
> -
> +out:
>   	rcu_read_unlock();
>   	return rc;
>   }
>