From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from goalie.tycho.ncsc.mil (goalie [144.51.242.250]) by tarius.tycho.ncsc.mil (8.14.4/8.14.4) with ESMTP id tAPJ76QU017823 for ; Wed, 25 Nov 2015 14:07:16 -0500 Received: by vkha189 with SMTP id a189so18341402vkh.2 for ; Tue, 24 Nov 2015 10:51:36 -0800 (PST) From: Paul Moore To: Stephen Smalley Cc: selinux@tycho.nsa.gov, bigon@debian.org, jeffv@google.com Subject: Re: [PATCH] selinux: fix bug in conditional rules handling Date: Tue, 24 Nov 2015 13:51:34 -0500 Message-ID: <1565543.7Ns8yjUNd2@sifl> In-Reply-To: <1448312861-3574-1-git-send-email-sds@tycho.nsa.gov> References: <1448312861-3574-1-git-send-email-sds@tycho.nsa.gov> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: On Monday, November 23, 2015 04:07:41 PM Stephen Smalley wrote: > commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced > a bug into the handling of conditional rules, skipping the processing > entirely when the caller does not provide an extended permissions (xperms) > structure. Access checks from userspace using /sys/fs/selinux/access > do not include such a structure since that interface does not presently > expose extended permission information. As a result, conditional rules > were being ignored entirely on userspace access requests, producing > denials when access was allowed by conditional rules in the policy. > Fix the bug by only skipping computation of extended permissions > in this situation, not the entire conditional rules processing. > > Reported-by: Laurent Bigonville > Signed-off-by: Stephen Smalley > --- > security/selinux/ss/conditional.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Merged into the SELinux stable branch, I'll push it to James tomorrow. > diff --git a/security/selinux/ss/conditional.c > b/security/selinux/ss/conditional.c index 18643bf..456e1a9 100644 > --- a/security/selinux/ss/conditional.c > +++ b/security/selinux/ss/conditional.c > @@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct > avtab_key *key, { > struct avtab_node *node; > > - if (!ctab || !key || !avd || !xperms) > + if (!ctab || !key || !avd) > return; > > for (node = avtab_search_node(ctab, key); node; > @@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct > avtab_key *key, if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) == > (node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED))) > avd->auditallow |= node->datum.u.data; > - if ((node->key.specified & AVTAB_ENABLED) && > + if (xperms && (node->key.specified & AVTAB_ENABLED) && > (node->key.specified & AVTAB_XPERMS)) > services_compute_xperms_drivers(xperms, node); > } -- paul moore www.paul-moore.com