Re: [PATCH v3] selinux: simplify mls_context_to_sid()
On 11/14/18 10:23 AM, Stephen Smalley wrote:
On 11/13/18 10:14 PM, Paul Moore wrote:
On Tue, Nov 13, 2018 at 4:10 PM Stephen Smalley
wrote:
On 11/12/18 6:44 AM, Ondrej Mosnacek wrote:
This function has only two callers, but only one of them actually needs
the special logic at the beginning. Factoring this logic out into
string_to_context_struct() allows us to drop the arguments 'oldc', 's',
and 'def_sid'.
Signed-off-by: Ondrej Mosnacek
---
Changes in v3:
- correct the comment about policy read lock
Changes in v2:
- also drop unneeded #include's from mls.c
security/selinux/ss/mls.c | 49
+-
security/selinux/ss/mls.h | 5 +---
security/selinux/ss/services.c | 32 +++---
3 files changed, 36 insertions(+), 50 deletions(-)
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 2fe459df3c85..d1da928a7e77 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -24,10 +24,7 @@
#include
#include
#include
-#include "sidtab.h"
#include "mls.h"
-#include "policydb.h"
-#include "services.h"
/*
* Return the length in bytes for the MLS fields of the
@@ -223,20 +220,12 @@ int mls_context_isvalid(struct policydb *p,
struct context *c)
* This function modifies the string in place, inserting
* NULL characters to terminate the MLS fields.
*
- * If a def_sid is provided and no MLS field is present,
- * copy the MLS field of the associated default context.
- * Used for upgraded to MLS systems where objects may lack
- * MLS fields.
- *
- * Policy read-lock must be held for sidtab lookup.
+ * Policy read-lock must be held for policy data lookup.
*
*/
int mls_context_to_sid(struct policydb *pol,
- char oldc,
char *scontext,
- struct context *context,
- struct sidtab *s,
- u32 def_sid)
+ struct context *context)
{
char *sensitivity, *cur_cat, *next_cat, *rngptr;
struct level_datum *levdatum;
@@ -244,29 +233,6 @@ int mls_context_to_sid(struct policydb *pol,
int l, rc, i;
char *rangep[2];
- if (!pol->mls_enabled) {
- if ((def_sid != SECSID_NULL && oldc) || (*scontext) ==
'\0')
- return 0;
- return -EINVAL;
- }
-
- /*
- * No MLS component to the security context, try and map to
- * default if provided.
- */
- if (!oldc) {
- struct context *defcon;
-
- if (def_sid == SECSID_NULL)
- return -EINVAL;
-
- defcon = sidtab_search(s, def_sid);
- if (!defcon)
- return -EINVAL;
-
- return mls_context_cpy(context, defcon);
- }
-
/*
* If we're dealing with a range, figure out where the two parts
* of the range begin.
@@ -364,14 +330,11 @@ int mls_from_string(struct policydb *p, char
*str, struct context *context,
return -EINVAL;
tmpstr = kstrdup(str, gfp_mask);
- if (!tmpstr) {
- rc = -ENOMEM;
- } else {
- rc = mls_context_to_sid(p, ':', tmpstr, context,
- NULL, SECSID_NULL);
- kfree(tmpstr);
- }
+ if (!tmpstr)
+ return -ENOMEM;
+ rc = mls_context_to_sid(p, tmpstr, context);
+ kfree(tmpstr);
return rc;
}
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 67093647576d..e2498f78e100 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -33,11 +33,8 @@ int mls_range_isvalid(struct policydb *p, struct
mls_range *r);
int mls_level_isvalid(struct policydb *p, struct mls_level *l);
int mls_context_to_sid(struct policydb *p,
- char oldc,
char *scontext,
- struct context *context,
- struct sidtab *s,
- u32 def_sid);
+ struct context *context);
int mls_from_string(struct policydb *p, char *str, struct context
*context,
gfp_t gfp_mask);
diff --git a/security/selinux/ss/services.c
b/security/selinux/ss/services.c
index 12e414394530..ccad4334f99d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1425,9 +1425,35 @@ static int string_to_context_struct(struct
policydb *pol,
ctx->type = typdatum->value;
- rc = mls_context_to_sid(pol, oldc, p, ctx, sidtabp, def_sid);
- if (rc)
- goto out;
+ if (!pol->mls_enabled) {
+ rc = -EINVAL;
+ if ((def_sid == SECSID_NULL || !oldc) && (*p) != '\0')
+ goto out;
I don't think this is your bug, but unless I'm mistaken, p could be OOB
and be dereferenced here. Seems to have been introduced by
95ffe194204ae3cef88d0b59be209204bbe9b3be. Likely not caught in testing
sin
Re: [PATCH v3] selinux: simplify mls_context_to_sid()
On 11/13/18 10:14 PM, Paul Moore wrote:
On Tue, Nov 13, 2018 at 4:10 PM Stephen Smalley wrote:
On 11/12/18 6:44 AM, Ondrej Mosnacek wrote:
This function has only two callers, but only one of them actually needs
the special logic at the beginning. Factoring this logic out into
string_to_context_struct() allows us to drop the arguments 'oldc', 's',
and 'def_sid'.
Signed-off-by: Ondrej Mosnacek
---
Changes in v3:
- correct the comment about policy read lock
Changes in v2:
- also drop unneeded #include's from mls.c
security/selinux/ss/mls.c | 49 +-
security/selinux/ss/mls.h | 5 +---
security/selinux/ss/services.c | 32 +++---
3 files changed, 36 insertions(+), 50 deletions(-)
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 2fe459df3c85..d1da928a7e77 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -24,10 +24,7 @@
#include
#include
#include
-#include "sidtab.h"
#include "mls.h"
-#include "policydb.h"
-#include "services.h"
/*
* Return the length in bytes for the MLS fields of the
@@ -223,20 +220,12 @@ int mls_context_isvalid(struct policydb *p, struct
context *c)
* This function modifies the string in place, inserting
* NULL characters to terminate the MLS fields.
*
- * If a def_sid is provided and no MLS field is present,
- * copy the MLS field of the associated default context.
- * Used for upgraded to MLS systems where objects may lack
- * MLS fields.
- *
- * Policy read-lock must be held for sidtab lookup.
+ * Policy read-lock must be held for policy data lookup.
*
*/
int mls_context_to_sid(struct policydb *pol,
-char oldc,
char *scontext,
-struct context *context,
-struct sidtab *s,
-u32 def_sid)
+struct context *context)
{
char *sensitivity, *cur_cat, *next_cat, *rngptr;
struct level_datum *levdatum;
@@ -244,29 +233,6 @@ int mls_context_to_sid(struct policydb *pol,
int l, rc, i;
char *rangep[2];
- if (!pol->mls_enabled) {
- if ((def_sid != SECSID_NULL && oldc) || (*scontext) == '\0')
- return 0;
- return -EINVAL;
- }
-
- /*
- * No MLS component to the security context, try and map to
- * default if provided.
- */
- if (!oldc) {
- struct context *defcon;
-
- if (def_sid == SECSID_NULL)
- return -EINVAL;
-
- defcon = sidtab_search(s, def_sid);
- if (!defcon)
- return -EINVAL;
-
- return mls_context_cpy(context, defcon);
- }
-
/*
* If we're dealing with a range, figure out where the two parts
* of the range begin.
@@ -364,14 +330,11 @@ int mls_from_string(struct policydb *p, char *str, struct
context *context,
return -EINVAL;
tmpstr = kstrdup(str, gfp_mask);
- if (!tmpstr) {
- rc = -ENOMEM;
- } else {
- rc = mls_context_to_sid(p, ':', tmpstr, context,
- NULL, SECSID_NULL);
- kfree(tmpstr);
- }
+ if (!tmpstr)
+ return -ENOMEM;
+ rc = mls_context_to_sid(p, tmpstr, context);
+ kfree(tmpstr);
return rc;
}
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 67093647576d..e2498f78e100 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -33,11 +33,8 @@ int mls_range_isvalid(struct policydb *p, struct mls_range
*r);
int mls_level_isvalid(struct policydb *p, struct mls_level *l);
int mls_context_to_sid(struct policydb *p,
-char oldc,
char *scontext,
-struct context *context,
-struct sidtab *s,
-u32 def_sid);
+struct context *context);
int mls_from_string(struct policydb *p, char *str, struct context *context,
gfp_t gfp_mask);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 12e414394530..ccad4334f99d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1425,9 +1425,35 @@ static int string_to_context_struct(struct policydb *pol,
ctx->type = typdatum->value;
- rc = mls_context_to_sid(pol, oldc, p, ctx, sidtabp, def_sid);
- if (rc)
- goto out;
+ if (!pol->mls_enabled) {
+ rc = -EINVAL;
+ if ((def_sid == SECSID_NULL || !oldc) && (*p) != '\0')
+ goto out;
I don't think this is your bug, but unless I'm mistaken, p could be OOB
and be dereferenced here. Seems to have been introduced by
95ffe194204ae3cef88d0b59be209204bbe9b3be. Likely not caught in testing
since both Linux distributions and Android enable MLS to use t
Re: [RFC PATCH 2/3] selinux: use separate table for initial SID lookup
On 11/14/18 4:45 AM, Ondrej Mosnacek wrote: On Tue, Nov 13, 2018 at 10:35 PM Stephen Smalley wrote: On 11/13/18 8:52 AM, Ondrej Mosnacek wrote: This patch is non-functional and moves handling of initial SIDs into a separate table. Note that the SIDs stored in the main table are now shifted by SECINITSID_NUM and converted to/from the actual SIDs transparently by helper functions. When you say "non-functional", you mean it doesn't make any functional changes, right? As opposed to leaving the kernel in a broken intermediate state until your 3rd patch? I mean it *should* be non-functional on its own, unless I made a mistake. I admit I didn't do very much testing on this patch, but I at least checked that I can boot the kernel, load a policy and that reported file contexts make sense. Commonly non-functional means "not working". I think you mean "this patch makes no functional changes". I think you need to double check that all references to state->ss->sidtab are preceded by a check of state->initialized since it could be NULL before first policy load and a system could come up with SELinux enabled but never load a policy. Well, the original code does not initialize the sidtab field with sidtab_init() either so state->ss->sidtab.htable would be NULL (or some garbage pointer) after boot and any use of sidtab would cause a GPF anyway. Prior to the patch, the sidtab is directly embedded in the selinux_ss, which is static and thus all fields are initialized to 0/NULL. But you could access the sidtab itself without any problem since it was not a pointer. Likewise for the policydb. When you change the sidtab to be a pointer, then you have to deal with the possibility that it will be NULL. So you might be introducing new situations where we would trigger a GPF. Looking at the places that reference the sidtab (which are all highlighted in the patch because of the switch to pointer type in the struct selinux_ss definition), these don't seem to check for state->initialized: - security_port_sid In this case, policydb->ocontexts[OCON_PORT] will be NULL at initialization, so c will be NULL and we will just set *out_sid to SECINITSID_PORT and return without ever accessing the sidtab. - security_ib_pkey_sid - security_ib_endport_sid - security_netif_sid - security_node_sid These are all similar. - security_sid_mls_copy, called from: This one checks state->initialized and returns if not set. - selinux_socket_unix_stream_connect (avc_has_perm is called before) - selinux_conn_sid, called from: - selinux_sctp_assoc_request (selinux_policycap_extsockclass is called before) - selinux_inet_conn_request - selinux_ip_postroute (selinux_policycap_netpeer is called before) - security_net_peersid_resolve, called from: This one should always return before taking the policy rwlock when !state->initialized because you can't have configured network labels without a policy IIUC (so xfrm_sid and/or nlbl_sid should be NULL), or regardless, policydb->mls_enabled will be 0 at this point. - selinux_skb_peerlbl_sid, called from: - selinux_socket_sock_rcv_skb (selinux_policycap_netpeer is called before) - selinux_socket_getpeersec_dgram - selinux_sctp_assoc_request (again) - selinux_inet_conn_request (again) - selinux_inet_conn_established - selinux_ip_forward (selinux_policycap_netpeer is called before) - selinux_ip_postroute (again) - selinux_audit_rule_match This one can't be called without a prior selinux_audit_rule_init, which checks state->initialized. - security_netlbl_secattr_to_sid, called from: This one checks state->initialized. - selinux_netlbl_sidlookup_cached, called from: - selinux_netlbl_skbuff_getsid (netlbl_enabled is called before) - selinux_netlbl_sock_rcv_skb (netlbl_enabled is called before) - security_netlbl_sid_to_secattr, called from: Ditto. - selinux_netlbl_sock_genattr, called from: - selinux_netlbl_socket_post_create, called from: - selinux_socket_post_create - selinux_netlbl_skbuff_setsid, called from: - selinux_ip_forward (again) - selinux_netlbl_sctp_assoc_request, called from: - selinux_sctp_assoc_request (again) - selinux_netlbl_inet_conn_request, called from: - selinux_inet_conn_request (again) I suppose in some (or all?) of these cases state->initialized might be implicitly always 1, but at least in these it wasn't immediately obvious to me. Either way, such cases would already be buggy before this patch, since they would happily access the policy structure (likely hitting some NULL/invalid pointers) and most likely also dereference the invalid htable pointer in sidtab. Aren't you still wasting a slot in the initial SIDs array, or did I miss something? Yes, I am, because the original code doesn't seem to guard against SECSID_NULL being loaded as initial SID into sidtab. I asked in the other thread whether this is considered a
Re: SELinux MLS for Apache Process
Many Thanks Stephen , it indeed worked !
Your support helped me a lot in doing my Masters thesis which is based on
SELinux MLS , I will definitely reach out to you which would require your
expertise .
Thanks again from Sri Lanka !!
On Thu, Nov 8, 2018 at 7:20 PM Stephen Smalley wrote:
> On 11/8/18 8:33 AM, Ishara Fernando wrote:
> > Dear Stephen ,
> >
> > Many thanks for the detailed information , it has been very useful .
> > Infact I have tested your steps in a similar environment (CentOS 6.10 ,
> > see versions below) as of yours in a Virtual machine based on
> > Virtualbox , I have reached to the step where the *selinux module is
> > installed* on doing the range transition to enforce httpd to run on
> > s4-s5:c1,c2 .
> >
> > Unfortunately I still see the range transition denied errors in the
> > audit logs (After installing the selinux module) and I do not see any
> > errors related to *httpd trying to perform writes* on various
> > directories/files that are labeled s0 as per your explanation .
> >
> > Kindly see the details below
> >
> > [root@msc-ishara-system1 ~]# sestatus -v
> > SELinux status: enabled
> > SELinuxfs mount:/selinux
> > Current mode: enforcing
> > Mode from config file: enforcing
> > Policy version: 24
> > Policy from config file:mls
> >
> > Process contexts:
> > Current context:staff_u:sysadm_r:sysadm_t:s4-s5:c1,c2
> > Init context: system_u:system_r:init_t:s0-s15:c0.c1023
> > /sbin/mingetty system_u:system_r:getty_t:s0-s15:c0.c1023
> > /usr/sbin/sshd system_u:system_r:sshd_t:s0-s15:c0.c1023
> >
> > File contexts:
> > Controlling term: staff_u:object_r:user_devpts_t:s4
> > /etc/passwd system_u:object_r:etc_t:s0
> > /etc/shadow system_u:object_r:shadow_t:s0
> > /bin/bash system_u:object_r:shell_exec_t:s0
> > /bin/login system_u:object_r:login_exec_t:s0
> > /bin/sh system_u:object_r:bin_t:s0 ->
> > system_u:object_r:shell_exec_t:s0
> > /sbin/agettysystem_u:object_r:getty_exec_t:s0
> > /sbin/init system_u:object_r:init_exec_t:s0
> > /sbin/mingetty system_u:object_r:getty_exec_t:s0
> > /usr/sbin/sshd system_u:object_r:sshd_exec_t:s0
> >
> >
> >
> > Dist: CentOS release 6.10 (Final)
> > Kernel : 2.6.32-754.6.3.el6.x86_64
> > SELinux MLS Policy RPM: selinux-policy-mls-3.7.19-312.el6.noarch
> > SELinux Policy version: 24
> >
> >
> > [root@msc-ishara-system1 ~]# id -Z
> > staff_u:sysadm_r:sysadm_t:s4-s5:c1,c2
>
> This is the problem - you switched levels before running run_init.
> And run_init tries to do an explicit transition to the context
> configured in /etc/selinux/$SELINUXTYPE/contexts/initrc_context. Just
> leave your shell in s0-s15:c0.1023, and let the range_transition rule
> handle transitioning httpd into s4-s5:c1,c2 for you automatically.
>
> >
> >
> > [root@msc-ishara-system1 ~]# ls -lZ /usr/sbin/httpd
> > -rwxr-xr-x. root root system_u:object_r:httpd_exec_t:s0 /usr/sbin/httpd
> >
> >
> >
> > [root@msc-ishara-system1 ~]# which run_init
> > /usr/sbin/run_init
> > [root@msc-ishara-system1 ~]# ls -lZ /usr/sbin/run_init
> > -rwxr-xr-x. root root system_u:object_r:run_init_exec_t:s0
> > /usr/sbin/run_init
> >
> >
> >
> > [root@msc-ishara-system1 /]# cat httpdtrans.te
> > policy_module(httpdtrans, 1.0)
> >
> > require {
> > type initrc_t;
> > type httpd_exec_t;
> > type httpd_t;
> > }
> >
> > range_transition initrc_t httpd_exec_t:process s4 - s5:c1,c2;
> >
> > mls_rangetrans_source(initrc_t)
> > mls_rangetrans_target(httpd_t)
> >
> >
> >
> > [root@msc-ishara-system1 /]# semodule -l | grep -i httpd
> > httpdtrans1.0
> >
> >
> >
> > [root@msc-ishara-system1 ~]# sesearch --type | grep -i initrc_t | grep
> > -i httpd_exec
> > type_transition initrc_t httpd_exec_t : process httpd_t;
> >
> >
> > [root@msc-ishara-system1 ~]# id -Z
> > staff_u:sysadm_r:sysadm_t:s4-s5:c1,c2
> >
> >
> > [root@msc-ishara-system1 ~]# run_init /etc/init.d/httpd start
> > Authenticating root.
> > Password:
> > execvp: Permission denied
> >
> >
> > [root@msc-ishara-system1 ~]# ausearch -i -m AVC -ts recent
> >
> > type=SYSCALL msg=audit(11/08/2018 18:32:36.457:160) : arch=x86_64
> > syscall=execve success=no exit=-13(Permission denied) a0=0x7ffd2309581a
> > a1=0x7ffd230949b0 a2=0x7ffd230949c8 a3=0x7ffd23094610 items=0 ppid=1802
> > pid=3074 auid=root uid=root gid=root euid=root suid=root fsuid=root
> > egid=root sgid=root fsgid=root tty=pts0 ses=3 comm=run_init
> > exe=/usr/sbin/run_init subj=staff_u:sysadm_r:run_init_t:s4-s5:c1,c2
> > key=(null)
> > type=AVC msg=audit(11/08/2018 18:32:36.457:160) :*avc: denied {
> > transition } f*or pid=3074 comm=run_init path=/etc/rc.d/init.d/httpd
> > dev=dm-0 ino=262967 scontext=s
Re: [RFC PATCH 2/3] selinux: use separate table for initial SID lookup
On Tue, Nov 13, 2018 at 10:35 PM Stephen Smalley wrote:
> On 11/13/18 8:52 AM, Ondrej Mosnacek wrote:
> > This patch is non-functional and moves handling of initial SIDs into a
> > separate table. Note that the SIDs stored in the main table are now
> > shifted by SECINITSID_NUM and converted to/from the actual SIDs
> > transparently by helper functions.
>
> When you say "non-functional", you mean it doesn't make any functional
> changes, right? As opposed to leaving the kernel in a broken
> intermediate state until your 3rd patch?
I mean it *should* be non-functional on its own, unless I made a
mistake. I admit I didn't do very much testing on this patch, but I at
least checked that I can boot the kernel, load a policy and that
reported file contexts make sense.
>
> I think you need to double check that all references to
> state->ss->sidtab are preceded by a check of state->initialized since it
> could be NULL before first policy load and a system could come up with
> SELinux enabled but never load a policy.
Well, the original code does not initialize the sidtab field with
sidtab_init() either so state->ss->sidtab.htable would be NULL (or
some garbage pointer) after boot and any use of sidtab would cause a
GPF anyway.
Looking at the places that reference the sidtab (which are all
highlighted in the patch because of the switch to pointer type in the
struct selinux_ss definition), these don't seem to check for
state->initialized:
- security_port_sid
- security_ib_pkey_sid
- security_ib_endport_sid
- security_netif_sid
- security_node_sid
- security_sid_mls_copy, called from:
- selinux_socket_unix_stream_connect (avc_has_perm is called before)
- selinux_conn_sid, called from:
- selinux_sctp_assoc_request (selinux_policycap_extsockclass is
called before)
- selinux_inet_conn_request
- selinux_ip_postroute (selinux_policycap_netpeer is called before)
- security_net_peersid_resolve, called from:
- selinux_skb_peerlbl_sid, called from:
- selinux_socket_sock_rcv_skb (selinux_policycap_netpeer is called before)
- selinux_socket_getpeersec_dgram
- selinux_sctp_assoc_request (again)
- selinux_inet_conn_request (again)
- selinux_inet_conn_established
- selinux_ip_forward (selinux_policycap_netpeer is called before)
- selinux_ip_postroute (again)
- selinux_audit_rule_match
- security_netlbl_secattr_to_sid, called from:
- selinux_netlbl_sidlookup_cached, called from:
- selinux_netlbl_skbuff_getsid (netlbl_enabled is called before)
- selinux_netlbl_sock_rcv_skb (netlbl_enabled is called before)
- security_netlbl_sid_to_secattr, called from:
- selinux_netlbl_sock_genattr, called from:
- selinux_netlbl_socket_post_create, called from:
- selinux_socket_post_create
- selinux_netlbl_skbuff_setsid, called from:
- selinux_ip_forward (again)
- selinux_netlbl_sctp_assoc_request, called from:
- selinux_sctp_assoc_request (again)
- selinux_netlbl_inet_conn_request, called from:
- selinux_inet_conn_request (again)
I suppose in some (or all?) of these cases state->initialized might be
implicitly always 1, but at least in these it wasn't immediately
obvious to me.
Either way, such cases would already be buggy before this patch, since
they would happily access the policy structure (likely hitting some
NULL/invalid pointers) and most likely also dereference the invalid
htable pointer in sidtab.
>
> Aren't you still wasting a slot in the initial SIDs array, or did I miss
> something?
Yes, I am, because the original code doesn't seem to guard against
SECSID_NULL being loaded as initial SID into sidtab. I asked in the
other thread whether this is considered a bug, but I didn't get a
reply, so I left it sort of bug-to-bug compatible for now... Anyway,
it doesn't seem to make much sense to keep that behavior so I will
probably just go ahead and add the check to policydb_load_isids() and
shrink the table. You can expect it to be resolved in the final
patchset.
>
> >
> > This change doesn't make much sense on its own, but it simplifies
> > further sidtab overhaul in a succeeding patch.
> >
> > Signed-off-by: Ondrej Mosnacek
> > ---
> > security/selinux/ss/policydb.c | 10 ++-
> > security/selinux/ss/services.c | 88 ++
> > security/selinux/ss/services.h | 2 +-
> > security/selinux/ss/sidtab.c | 158 +++--
> > security/selinux/ss/sidtab.h | 14 +--
> > 5 files changed, 162 insertions(+), 110 deletions(-)
> >
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index f4eadd3f7350..21e4bdbcf994 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -909,13 +909,21 @@ int policydb_load_isids(struct policydb *p, struct
> > sidtab *s)
> > if (!c->context[0].user) {
> > pr_err("SELinux: SID %s was never defined.\n",
> > c->u.name);
> > +
Re: [PATCH v3] selinux: simplify mls_context_to_sid()
On Wed, Nov 14, 2018 at 4:14 AM Paul Moore wrote:
> On Tue, Nov 13, 2018 at 4:10 PM Stephen Smalley wrote:
> > On 11/12/18 6:44 AM, Ondrej Mosnacek wrote:
> > > This function has only two callers, but only one of them actually needs
> > > the special logic at the beginning. Factoring this logic out into
> > > string_to_context_struct() allows us to drop the arguments 'oldc', 's',
> > > and 'def_sid'.
> > >
> > > Signed-off-by: Ondrej Mosnacek
> > > ---
> > >
> > > Changes in v3:
> > > - correct the comment about policy read lock
> > >
> > > Changes in v2:
> > > - also drop unneeded #include's from mls.c
> > >
> > > security/selinux/ss/mls.c | 49 +-
> > > security/selinux/ss/mls.h | 5 +---
> > > security/selinux/ss/services.c | 32 +++---
> > > 3 files changed, 36 insertions(+), 50 deletions(-)
> > >
> > > diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> > > index 2fe459df3c85..d1da928a7e77 100644
> > > --- a/security/selinux/ss/mls.c
> > > +++ b/security/selinux/ss/mls.c
> > > @@ -24,10 +24,7 @@
> > > #include
> > > #include
> > > #include
> > > -#include "sidtab.h"
> > > #include "mls.h"
> > > -#include "policydb.h"
> > > -#include "services.h"
> > >
> > > /*
> > >* Return the length in bytes for the MLS fields of the
> > > @@ -223,20 +220,12 @@ int mls_context_isvalid(struct policydb *p, struct
> > > context *c)
> > >* This function modifies the string in place, inserting
> > >* NULL characters to terminate the MLS fields.
> > >*
> > > - * If a def_sid is provided and no MLS field is present,
> > > - * copy the MLS field of the associated default context.
> > > - * Used for upgraded to MLS systems where objects may lack
> > > - * MLS fields.
> > > - *
> > > - * Policy read-lock must be held for sidtab lookup.
> > > + * Policy read-lock must be held for policy data lookup.
> > >*
> > >*/
> > > int mls_context_to_sid(struct policydb *pol,
> > > -char oldc,
> > > char *scontext,
> > > -struct context *context,
> > > -struct sidtab *s,
> > > -u32 def_sid)
> > > +struct context *context)
> > > {
> > > char *sensitivity, *cur_cat, *next_cat, *rngptr;
> > > struct level_datum *levdatum;
> > > @@ -244,29 +233,6 @@ int mls_context_to_sid(struct policydb *pol,
> > > int l, rc, i;
> > > char *rangep[2];
> > >
> > > - if (!pol->mls_enabled) {
> > > - if ((def_sid != SECSID_NULL && oldc) || (*scontext) == '\0')
> > > - return 0;
> > > - return -EINVAL;
> > > - }
> > > -
> > > - /*
> > > - * No MLS component to the security context, try and map to
> > > - * default if provided.
> > > - */
> > > - if (!oldc) {
> > > - struct context *defcon;
> > > -
> > > - if (def_sid == SECSID_NULL)
> > > - return -EINVAL;
> > > -
> > > - defcon = sidtab_search(s, def_sid);
> > > - if (!defcon)
> > > - return -EINVAL;
> > > -
> > > - return mls_context_cpy(context, defcon);
> > > - }
> > > -
> > > /*
> > >* If we're dealing with a range, figure out where the two parts
> > >* of the range begin.
> > > @@ -364,14 +330,11 @@ int mls_from_string(struct policydb *p, char *str,
> > > struct context *context,
> > > return -EINVAL;
> > >
> > > tmpstr = kstrdup(str, gfp_mask);
> > > - if (!tmpstr) {
> > > - rc = -ENOMEM;
> > > - } else {
> > > - rc = mls_context_to_sid(p, ':', tmpstr, context,
> > > - NULL, SECSID_NULL);
> > > - kfree(tmpstr);
> > > - }
> > > + if (!tmpstr)
> > > + return -ENOMEM;
> > >
> > > + rc = mls_context_to_sid(p, tmpstr, context);
> > > + kfree(tmpstr);
> > > return rc;
> > > }
> > >
> > > diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
> > > index 67093647576d..e2498f78e100 100644
> > > --- a/security/selinux/ss/mls.h
> > > +++ b/security/selinux/ss/mls.h
> > > @@ -33,11 +33,8 @@ int mls_range_isvalid(struct policydb *p, struct
> > > mls_range *r);
> > > int mls_level_isvalid(struct policydb *p, struct mls_level *l);
> > >
> > > int mls_context_to_sid(struct policydb *p,
> > > -char oldc,
> > > char *scontext,
> > > -struct context *context,
> > > -struct sidtab *s,
> > > -u32 def_sid);
> > > +struct context *context);
> > >
> > > int mls_from_string(struct policydb *p, char *str, struct context
> > > *context,
> > > gfp_t gfp_mask);
> > > diff --git a/security/selinux/ss/services.c
> > > b/security/selinux/ss/services.c
> > > index 12e414394530..
