From 2e5aa86609ec1cf37bcc204fd7ba6c24c2f49fec Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:38 -0400 Subject: lsm: split the xfrm_state_alloc_security() hook implementation The xfrm_state_alloc_security() LSM hook implementation is really a multiplexed hook with two different behaviors depending on the arguments passed to it by the caller. This patch splits the LSM hook implementation into two new hook implementations, which match the LSM hooks in the rest of the kernel: * xfrm_state_alloc * xfrm_state_alloc_acquire Also included in this patch are the necessary changes to the SELinux code; no other LSMs are affected. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/hooks.c | 3 +- security/selinux/include/xfrm.h | 4 +- security/selinux/xfrm.c | 199 ++++++++++++++++++---------------------- 3 files changed, 95 insertions(+), 111 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5c6f2cd2d095..d3555bdf66af 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5708,7 +5708,8 @@ static struct security_operations selinux_ops = { .xfrm_policy_clone_security = selinux_xfrm_policy_clone, .xfrm_policy_free_security = selinux_xfrm_policy_free, .xfrm_policy_delete_security = selinux_xfrm_policy_delete, - .xfrm_state_alloc_security = selinux_xfrm_state_alloc, + .xfrm_state_alloc = selinux_xfrm_state_alloc, + .xfrm_state_alloc_acquire = selinux_xfrm_state_alloc_acquire, .xfrm_state_free_security = selinux_xfrm_state_free, .xfrm_state_delete_security = selinux_xfrm_state_delete, .xfrm_policy_lookup = selinux_xfrm_policy_lookup, diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index 65f67cb0aefb..f2a2314aac1a 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -16,7 +16,9 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx); int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx); int selinux_xfrm_state_alloc(struct xfrm_state *x, - struct xfrm_user_sec_ctx *sec_ctx, u32 secid); + struct xfrm_user_sec_ctx *uctx); +int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, + struct xfrm_sec_ctx *polsec, u32 secid); void selinux_xfrm_state_free(struct xfrm_state *x); int selinux_xfrm_state_delete(struct xfrm_state *x); int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir); diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index d03081886214..07ae0c06dfc3 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -73,6 +73,54 @@ static inline int selinux_authorizable_xfrm(struct xfrm_state *x) return selinux_authorizable_ctx(x->security); } +/* + * Allocates a xfrm_sec_state and populates it using the supplied security + * xfrm_user_sec_ctx context. + */ +static int selinux_xfrm_alloc_user(struct xfrm_sec_ctx **ctxp, + struct xfrm_user_sec_ctx *uctx) +{ + int rc; + const struct task_security_struct *tsec = current_security(); + struct xfrm_sec_ctx *ctx = NULL; + u32 str_len; + + if (ctxp == NULL || uctx == NULL || + uctx->ctx_doi != XFRM_SC_DOI_LSM || + uctx->ctx_alg != XFRM_SC_ALG_SELINUX) + return -EINVAL; + + str_len = uctx->ctx_len; + if (str_len >= PAGE_SIZE) + return -ENOMEM; + + ctx = kmalloc(sizeof(*ctx) + str_len + 1, GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->ctx_doi = XFRM_SC_DOI_LSM; + ctx->ctx_alg = XFRM_SC_ALG_SELINUX; + ctx->ctx_len = str_len; + memcpy(ctx->ctx_str, &uctx[1], str_len); + ctx->ctx_str[str_len] = '\0'; + rc = security_context_to_sid(ctx->ctx_str, str_len, &ctx->ctx_sid); + if (rc) + goto err; + + rc = avc_has_perm(tsec->sid, ctx->ctx_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT, NULL); + if (rc) + goto err; + + *ctxp = ctx; + atomic_inc(&selinux_xfrm_refcount); + return 0; + +err: + kfree(ctx); + return rc; +} + /* * LSM hook implementation that authorizes that a flow can use * a xfrm policy rule. @@ -190,96 +238,6 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall) return 0; } -/* - * Security blob allocation for xfrm_policy and xfrm_state - * CTX does not have a meaningful value on input - */ -static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp, - struct xfrm_user_sec_ctx *uctx, u32 sid) -{ - int rc = 0; - const struct task_security_struct *tsec = current_security(); - struct xfrm_sec_ctx *ctx = NULL; - char *ctx_str = NULL; - u32 str_len; - - BUG_ON(uctx && sid); - - if (!uctx) - goto not_from_user; - - if (uctx->ctx_alg != XFRM_SC_ALG_SELINUX) - return -EINVAL; - - str_len = uctx->ctx_len; - if (str_len >= PAGE_SIZE) - return -ENOMEM; - - *ctxp = ctx = kmalloc(sizeof(*ctx) + - str_len + 1, - GFP_KERNEL); - - if (!ctx) - return -ENOMEM; - - ctx->ctx_doi = uctx->ctx_doi; - ctx->ctx_len = str_len; - ctx->ctx_alg = uctx->ctx_alg; - - memcpy(ctx->ctx_str, - uctx+1, - str_len); - ctx->ctx_str[str_len] = 0; - rc = security_context_to_sid(ctx->ctx_str, - str_len, - &ctx->ctx_sid); - - if (rc) - goto out; - - /* - * Does the subject have permission to set security context? - */ - rc = avc_has_perm(tsec->sid, ctx->ctx_sid, - SECCLASS_ASSOCIATION, - ASSOCIATION__SETCONTEXT, NULL); - if (rc) - goto out; - - return rc; - -not_from_user: - rc = security_sid_to_context(sid, &ctx_str, &str_len); - if (rc) - goto out; - - *ctxp = ctx = kmalloc(sizeof(*ctx) + - str_len, - GFP_ATOMIC); - - if (!ctx) { - rc = -ENOMEM; - goto out; - } - - ctx->ctx_doi = XFRM_SC_DOI_LSM; - ctx->ctx_alg = XFRM_SC_ALG_SELINUX; - ctx->ctx_sid = sid; - ctx->ctx_len = str_len; - memcpy(ctx->ctx_str, - ctx_str, - str_len); - - goto out2; - -out: - *ctxp = NULL; - kfree(ctx); -out2: - kfree(ctx_str); - return rc; -} - /* * LSM hook implementation that allocs and transfers uctx spec to * xfrm_policy. @@ -287,15 +245,7 @@ out2: int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, struct xfrm_user_sec_ctx *uctx) { - int err; - - BUG_ON(!uctx); - - err = selinux_xfrm_sec_ctx_alloc(ctxp, uctx, 0); - if (err == 0) - atomic_inc(&selinux_xfrm_refcount); - - return err; + return selinux_xfrm_alloc_user(ctxp, uctx); } @@ -347,20 +297,51 @@ int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx) } /* - * LSM hook implementation that allocs and transfers sec_ctx spec to - * xfrm_state. + * LSM hook implementation that allocates a xfrm_sec_state, populates it using + * the supplied security context, and assigns it to the xfrm_state. + */ +int selinux_xfrm_state_alloc(struct xfrm_state *x, + struct xfrm_user_sec_ctx *uctx) +{ + return selinux_xfrm_alloc_user(&x->security, uctx); +} + +/* + * LSM hook implementation that allocates a xfrm_sec_state and populates based + * on a secid. */ -int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *uctx, - u32 secid) +int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, + struct xfrm_sec_ctx *polsec, u32 secid) { - int err; + int rc; + struct xfrm_sec_ctx *ctx; + char *ctx_str = NULL; + int str_len; - BUG_ON(!x); + if (!polsec) + return 0; - err = selinux_xfrm_sec_ctx_alloc(&x->security, uctx, secid); - if (err == 0) - atomic_inc(&selinux_xfrm_refcount); - return err; + if (secid == 0) + return -EINVAL; + + rc = security_sid_to_context(secid, &ctx_str, &str_len); + if (rc) + return rc; + + ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC); + if (!ctx) + return -ENOMEM; + + ctx->ctx_doi = XFRM_SC_DOI_LSM; + ctx->ctx_alg = XFRM_SC_ALG_SELINUX; + ctx->ctx_sid = secid; + ctx->ctx_len = str_len; + memcpy(ctx->ctx_str, ctx_str, str_len); + kfree(ctx_str); + + x->security = ctx; + atomic_inc(&selinux_xfrm_refcount); + return 0; } /* -- cgit v1.2.3-55-g7522 From ccf17cc4b81537c29f0d5950b38b5548b6cb5858 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:39 -0400 Subject: selinux: cleanup and consolidate the XFRM alloc/clone/delete/free code The SELinux labeled IPsec code state management functions have been long neglected and could use some cleanup and consolidation. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/xfrm.c | 71 ++++++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 31 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 07ae0c06dfc3..f8d71262b45d 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -121,6 +121,33 @@ err: return rc; } +/* + * Free the xfrm_sec_ctx structure. + */ +static void selinux_xfrm_free(struct xfrm_sec_ctx *ctx) +{ + if (!ctx) + return; + + atomic_dec(&selinux_xfrm_refcount); + kfree(ctx); +} + +/* + * Authorize the deletion of a labeled SA or policy rule. + */ +static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx) +{ + const struct task_security_struct *tsec = current_security(); + + if (!ctx) + return 0; + + return avc_has_perm(tsec->sid, ctx->ctx_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT, + NULL); +} + /* * LSM hook implementation that authorizes that a flow can use * a xfrm policy rule. @@ -258,17 +285,16 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, { struct xfrm_sec_ctx *new_ctx; - if (old_ctx) { - new_ctx = kmalloc(sizeof(*old_ctx) + old_ctx->ctx_len, - GFP_ATOMIC); - if (!new_ctx) - return -ENOMEM; + if (!old_ctx) + return 0; + + new_ctx = kmalloc(sizeof(*old_ctx) + old_ctx->ctx_len, GFP_ATOMIC); + if (!new_ctx) + return -ENOMEM; + memcpy(new_ctx, old_ctx, sizeof(*old_ctx) + old_ctx->ctx_len); + atomic_inc(&selinux_xfrm_refcount); + *new_ctxp = new_ctx; - memcpy(new_ctx, old_ctx, sizeof(*new_ctx)); - memcpy(new_ctx->ctx_str, old_ctx->ctx_str, new_ctx->ctx_len); - atomic_inc(&selinux_xfrm_refcount); - *new_ctxp = new_ctx; - } return 0; } @@ -277,8 +303,7 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, */ void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx) { - atomic_dec(&selinux_xfrm_refcount); - kfree(ctx); + selinux_xfrm_free(ctx); } /* @@ -286,14 +311,7 @@ void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx) */ int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx) { - const struct task_security_struct *tsec = current_security(); - - if (!ctx) - return 0; - - return avc_has_perm(tsec->sid, ctx->ctx_sid, - SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT, - NULL); + return selinux_xfrm_delete(ctx); } /* @@ -349,8 +367,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, */ void selinux_xfrm_state_free(struct xfrm_state *x) { - atomic_dec(&selinux_xfrm_refcount); - kfree(x->security); + selinux_xfrm_free(x->security); } /* @@ -358,15 +375,7 @@ void selinux_xfrm_state_free(struct xfrm_state *x) */ int selinux_xfrm_state_delete(struct xfrm_state *x) { - const struct task_security_struct *tsec = current_security(); - struct xfrm_sec_ctx *ctx = x->security; - - if (!ctx) - return 0; - - return avc_has_perm(tsec->sid, ctx->ctx_sid, - SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT, - NULL); + return selinux_xfrm_delete(x->security); } /* -- cgit v1.2.3-55-g7522 From 96484348ad712aed2c6ad7229b848bc1bf6165a8 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:39 -0400 Subject: selinux: cleanup selinux_xfrm_policy_lookup() and selinux_xfrm_state_pol_flow_match() Do some basic simplification and comment reformatting. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/xfrm.c | 54 +++++++++++++++++-------------------------------- 1 file changed, 18 insertions(+), 36 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index f8d71262b45d..4a7ba4ac487e 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -155,42 +155,30 @@ static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx) int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir) { int rc; - u32 sel_sid; - /* Context sid is either set to label or ANY_ASSOC */ - if (ctx) { - if (!selinux_authorizable_ctx(ctx)) - return -EINVAL; - - sel_sid = ctx->ctx_sid; - } else - /* - * All flows should be treated as polmatch'ing an - * otherwise applicable "non-labeled" policy. This - * would prevent inadvertent "leaks". - */ + /* All flows should be treated as polmatch'ing an otherwise applicable + * "non-labeled" policy. This would prevent inadvertent "leaks". */ + if (!ctx) return 0; - rc = avc_has_perm(fl_secid, sel_sid, SECCLASS_ASSOCIATION, - ASSOCIATION__POLMATCH, - NULL); - - if (rc == -EACCES) - return -ESRCH; + /* Context sid is either set to label or ANY_ASSOC */ + if (!selinux_authorizable_ctx(ctx)) + return -EINVAL; - return rc; + rc = avc_has_perm(fl_secid, ctx->ctx_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__POLMATCH, NULL); + return (rc == -EACCES ? -ESRCH : rc); } /* * LSM hook implementation that authorizes that a state matches * the given policy, flow combo. */ - -int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, - const struct flowi *fl) +int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, + struct xfrm_policy *xp, + const struct flowi *fl) { u32 state_sid; - int rc; if (!xp->security) if (x->security) @@ -213,18 +201,12 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy * if (fl->flowi_secid != state_sid) return 0; - rc = avc_has_perm(fl->flowi_secid, state_sid, SECCLASS_ASSOCIATION, - ASSOCIATION__SENDTO, - NULL)? 0:1; - - /* - * We don't need a separate SA Vs. policy polmatch check - * since the SA is now of the same label as the flow and - * a flow Vs. policy polmatch check had already happened - * in selinux_xfrm_policy_lookup() above. - */ - - return rc; + /* We don't need a separate SA Vs. policy polmatch check since the SA + * is now of the same label as the flow and a flow Vs. policy polmatch + * check had already happened in selinux_xfrm_policy_lookup() above. */ + return (avc_has_perm(fl->flowi_secid, state_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__SENDTO, + NULL) ? 0 : 1); } /* -- cgit v1.2.3-55-g7522 From eef9b41622f2f09e824fb4e7356b42bddada6623 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:39 -0400 Subject: selinux: cleanup selinux_xfrm_sock_rcv_skb() and selinux_xfrm_postroute_last() Some basic simplification and comment reformatting. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/include/xfrm.h | 17 +++++---- security/selinux/xfrm.c | 85 ++++++++++++++++------------------------- 2 files changed, 42 insertions(+), 60 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index f2a2314aac1a..b463f210f1af 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -44,10 +44,10 @@ static inline int selinux_xfrm_enabled(void) return (atomic_read(&selinux_xfrm_refcount) > 0); } -int selinux_xfrm_sock_rcv_skb(u32 sid, struct sk_buff *skb, - struct common_audit_data *ad); -int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb, - struct common_audit_data *ad, u8 proto); +int selinux_xfrm_sock_rcv_skb(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad); +int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad, u8 proto); int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall); static inline void selinux_xfrm_notify_policyload(void) @@ -61,14 +61,15 @@ static inline int selinux_xfrm_enabled(void) return 0; } -static inline int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb, - struct common_audit_data *ad) +static inline int selinux_xfrm_sock_rcv_skb(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad) { return 0; } -static inline int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb, - struct common_audit_data *ad, u8 proto) +static inline int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad, + u8 proto) { return 0; } diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 4a7ba4ac487e..1f6c6e619e98 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -367,14 +367,12 @@ int selinux_xfrm_state_delete(struct xfrm_state *x) * we need to check for unlabelled access since this may not have * gone thru the IPSec process. */ -int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb, - struct common_audit_data *ad) +int selinux_xfrm_sock_rcv_skb(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad) { - int i, rc = 0; - struct sec_path *sp; - u32 sel_sid = SECINITSID_UNLABELED; - - sp = skb->sp; + int i; + struct sec_path *sp = skb->sp; + u32 peer_sid = SECINITSID_UNLABELED; if (sp) { for (i = 0; i < sp->len; i++) { @@ -382,23 +380,17 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb, if (x && selinux_authorizable_xfrm(x)) { struct xfrm_sec_ctx *ctx = x->security; - sel_sid = ctx->ctx_sid; + peer_sid = ctx->ctx_sid; break; } } } - /* - * This check even when there's no association involved is - * intended, according to Trent Jaeger, to make sure a - * process can't engage in non-ipsec communication unless - * explicitly allowed by policy. - */ - - rc = avc_has_perm(isec_sid, sel_sid, SECCLASS_ASSOCIATION, - ASSOCIATION__RECVFROM, ad); - - return rc; + /* This check even when there's no association involved is intended, + * according to Trent Jaeger, to make sure a process can't engage in + * non-IPsec communication unless explicitly allowed by policy. */ + return avc_has_perm(sk_sid, peer_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__RECVFROM, ad); } /* @@ -408,49 +400,38 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb, * If we do have a authorizable security association, then it has already been * checked in the selinux_xfrm_state_pol_flow_match hook above. */ -int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb, - struct common_audit_data *ad, u8 proto) +int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb, + struct common_audit_data *ad, u8 proto) { struct dst_entry *dst; - int rc = 0; - - dst = skb_dst(skb); - - if (dst) { - struct dst_entry *dst_test; - - for (dst_test = dst; dst_test != NULL; - dst_test = dst_test->child) { - struct xfrm_state *x = dst_test->xfrm; - - if (x && selinux_authorizable_xfrm(x)) - goto out; - } - } switch (proto) { case IPPROTO_AH: case IPPROTO_ESP: case IPPROTO_COMP: - /* - * We should have already seen this packet once before - * it underwent xfrm(s). No need to subject it to the - * unlabeled check. - */ - goto out; + /* We should have already seen this packet once before it + * underwent xfrm(s). No need to subject it to the unlabeled + * check. */ + return 0; default: break; } - /* - * This check even when there's no association involved is - * intended, according to Trent Jaeger, to make sure a - * process can't engage in non-ipsec communication unless - * explicitly allowed by policy. - */ + dst = skb_dst(skb); + if (dst) { + struct dst_entry *iter; - rc = avc_has_perm(isec_sid, SECINITSID_UNLABELED, SECCLASS_ASSOCIATION, - ASSOCIATION__SENDTO, ad); -out: - return rc; + for (iter = dst; iter != NULL; iter = iter->child) { + struct xfrm_state *x = iter->xfrm; + + if (x && selinux_authorizable_xfrm(x)) + return 0; + } + } + + /* This check even when there's no association involved is intended, + * according to Trent Jaeger, to make sure a process can't engage in + * non-IPsec communication unless explicitly allowed by policy. */ + return avc_has_perm(sk_sid, SECINITSID_UNLABELED, + SECCLASS_ASSOCIATION, ASSOCIATION__SENDTO, ad); } -- cgit v1.2.3-55-g7522 From 4baabeec2a061fe771f9fcc01c61204a2ee2f608 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:39 -0400 Subject: selinux: cleanup some comment and whitespace issues in the XFRM code Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/xfrm.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 1f6c6e619e98..00801cef1dd9 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -56,7 +56,7 @@ atomic_t selinux_xfrm_refcount = ATOMIC_INIT(0); /* - * Returns true if an LSM/SELinux context + * Returns true if the context is an LSM/SELinux context. */ static inline int selinux_authorizable_ctx(struct xfrm_sec_ctx *ctx) { @@ -66,7 +66,7 @@ static inline int selinux_authorizable_ctx(struct xfrm_sec_ctx *ctx) } /* - * Returns true if the xfrm contains a security blob for SELinux + * Returns true if the xfrm contains a security blob for SELinux. */ static inline int selinux_authorizable_xfrm(struct xfrm_state *x) { @@ -149,8 +149,8 @@ static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx) } /* - * LSM hook implementation that authorizes that a flow can use - * a xfrm policy rule. + * LSM hook implementation that authorizes that a flow can use a xfrm policy + * rule. */ int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir) { @@ -213,7 +213,6 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, * LSM hook implementation that checks and/or returns the xfrm sid for the * incoming packet. */ - int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall) { struct sec_path *sp; @@ -248,8 +247,7 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall) } /* - * LSM hook implementation that allocs and transfers uctx spec to - * xfrm_policy. + * LSM hook implementation that allocs and transfers uctx spec to xfrm_policy. */ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, struct xfrm_user_sec_ctx *uctx) @@ -257,10 +255,9 @@ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, return selinux_xfrm_alloc_user(ctxp, uctx); } - /* - * LSM hook implementation that copies security data structure from old to - * new for policy cloning. + * LSM hook implementation that copies security data structure from old to new + * for policy cloning. */ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, struct xfrm_sec_ctx **new_ctxp) @@ -352,9 +349,9 @@ void selinux_xfrm_state_free(struct xfrm_state *x) selinux_xfrm_free(x->security); } - /* - * LSM hook implementation that authorizes deletion of labeled SAs. - */ +/* + * LSM hook implementation that authorizes deletion of labeled SAs. + */ int selinux_xfrm_state_delete(struct xfrm_state *x) { return selinux_xfrm_delete(x->security); -- cgit v1.2.3-55-g7522 From e21936958000a4f2298193b3875b707fbcbc8f7b Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:40 -0400 Subject: selinux: cleanup selinux_xfrm_decode_session() Some basic simplification. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/xfrm.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 00801cef1dd9..425b9f91d755 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -215,34 +215,35 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, */ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall) { + u32 sid_session = SECSID_NULL; struct sec_path *sp; - *sid = SECSID_NULL; - if (skb == NULL) - return 0; + goto out; sp = skb->sp; if (sp) { - int i, sid_set = 0; + int i; - for (i = sp->len-1; i >= 0; i--) { + for (i = sp->len - 1; i >= 0; i--) { struct xfrm_state *x = sp->xvec[i]; if (selinux_authorizable_xfrm(x)) { struct xfrm_sec_ctx *ctx = x->security; - if (!sid_set) { - *sid = ctx->ctx_sid; - sid_set = 1; - + if (sid_session == SECSID_NULL) { + sid_session = ctx->ctx_sid; if (!ckall) - break; - } else if (*sid != ctx->ctx_sid) + goto out; + } else if (sid_session != ctx->ctx_sid) { + *sid = SECSID_NULL; return -EINVAL; + } } } } +out: + *sid = sid_session; return 0; } -- cgit v1.2.3-55-g7522 From d1b17b09f390369271ecfbc7e4a4d0d0dcff1b1b Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:40 -0400 Subject: selinux: cleanup the XFRM header Remove the unused get_sock_isec() function and do some formatting fixes. Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/include/xfrm.h | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index b463f210f1af..60774bcf61b3 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -10,7 +10,7 @@ #include int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, - struct xfrm_user_sec_ctx *sec_ctx); + struct xfrm_user_sec_ctx *uctx); int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, struct xfrm_sec_ctx **new_ctxp); void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx); @@ -23,18 +23,8 @@ void selinux_xfrm_state_free(struct xfrm_state *x); int selinux_xfrm_state_delete(struct xfrm_state *x); int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir); int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, - struct xfrm_policy *xp, const struct flowi *fl); - -/* - * Extract the security blob from the sock (it's actually on the socket) - */ -static inline struct inode_security_struct *get_sock_isec(struct sock *sk) -{ - if (!sk->sk_socket) - return NULL; - - return SOCK_INODE(sk->sk_socket)->i_security; -} + struct xfrm_policy *xp, + const struct flowi *fl); #ifdef CONFIG_SECURITY_NETWORK_XFRM extern atomic_t selinux_xfrm_refcount; @@ -74,7 +64,8 @@ static inline int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb, return 0; } -static inline int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall) +static inline int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, + int ckall) { *sid = SECSID_NULL; return 0; -- cgit v1.2.3-55-g7522 From bed4d7efb31fd81b3a3c83dc8540197cd0fe81c0 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:40 -0400 Subject: selinux: remove the BUG_ON() from selinux_skb_xfrm_sid() Remove the BUG_ON() from selinux_skb_xfrm_sid() and propogate the error code up to the caller. Also check the return values in the only caller function, selinux_skb_peerlbl_sid(). Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/hooks.c | 8 ++++++-- security/selinux/include/xfrm.h | 5 ++--- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d3555bdf66af..4bc068b3773d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3722,8 +3722,12 @@ static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u16 family, u32 *sid) u32 nlbl_sid; u32 nlbl_type; - selinux_skb_xfrm_sid(skb, &xfrm_sid); - selinux_netlbl_skbuff_getsid(skb, family, &nlbl_type, &nlbl_sid); + err = selinux_skb_xfrm_sid(skb, &xfrm_sid); + if (unlikely(err)) + return -EACCES; + err = selinux_netlbl_skbuff_getsid(skb, family, &nlbl_type, &nlbl_sid); + if (unlikely(err)) + return -EACCES; err = security_net_peersid_resolve(nlbl_sid, nlbl_type, xfrm_sid, sid); if (unlikely(err)) { diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index 60774bcf61b3..7605251936f5 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -76,10 +76,9 @@ static inline void selinux_xfrm_notify_policyload(void) } #endif -static inline void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid) +static inline int selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid) { - int err = selinux_xfrm_decode_session(skb, sid, 0); - BUG_ON(err); + return selinux_xfrm_decode_session(skb, sid, 0); } #endif /* _SELINUX_XFRM_H_ */ -- cgit v1.2.3-55-g7522 From fee7114298cf54bbd221cdb2ab49738be8b94f4c Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Tue, 23 Jul 2013 17:38:41 -0400 Subject: SELinux: Reduce overhead of mls_level_isvalid() function call While running the high_systime workload of the AIM7 benchmark on a 2-socket 12-core Westmere x86-64 machine running 3.10-rc4 kernel (with HT on), it was found that a pretty sizable amount of time was spent in the SELinux code. Below was the perf trace of the "perf record -a -s" of a test run at 1500 users: 5.04% ls [kernel.kallsyms] [k] ebitmap_get_bit 1.96% ls [kernel.kallsyms] [k] mls_level_isvalid 1.95% ls [kernel.kallsyms] [k] find_next_bit The ebitmap_get_bit() was the hottest function in the perf-report output. Both the ebitmap_get_bit() and find_next_bit() functions were, in fact, called by mls_level_isvalid(). As a result, the mls_level_isvalid() call consumed 8.95% of the total CPU time of all the 24 virtual CPUs which is quite a lot. The majority of the mls_level_isvalid() function invocations come from the socket creation system call. Looking at the mls_level_isvalid() function, it is checking to see if all the bits set in one of the ebitmap structure are also set in another one as well as the highest set bit is no bigger than the one specified by the given policydb data structure. It is doing it in a bit-by-bit manner. So if the ebitmap structure has many bits set, the iteration loop will be done many times. The current code can be rewritten to use a similar algorithm as the ebitmap_contains() function with an additional check for the highest set bit. The ebitmap_contains() function was extended to cover an optional additional check for the highest set bit, and the mls_level_isvalid() function was modified to call ebitmap_contains(). With that change, the perf trace showed that the used CPU time drop down to just 0.08% (ebitmap_contains + mls_level_isvalid) of the total which is about 100X less than before. 0.07% ls [kernel.kallsyms] [k] ebitmap_contains 0.05% ls [kernel.kallsyms] [k] ebitmap_get_bit 0.01% ls [kernel.kallsyms] [k] mls_level_isvalid 0.01% ls [kernel.kallsyms] [k] find_next_bit The remaining ebitmap_get_bit() and find_next_bit() functions calls are made by other kernel routines as the new mls_level_isvalid() function will not call them anymore. This patch also improves the high_systime AIM7 benchmark result, though the improvement is not as impressive as is suggested by the reduction in CPU time spent in the ebitmap functions. The table below shows the performance change on the 2-socket x86-64 system (with HT on) mentioned above. +--------------+---------------+----------------+-----------------+ | Workload | mean % change | mean % change | mean % change | | | 10-100 users | 200-1000 users | 1100-2000 users | +--------------+---------------+----------------+-----------------+ | high_systime | +0.1% | +0.9% | +2.6% | +--------------+---------------+----------------+-----------------+ Signed-off-by: Waiman Long Acked-by: Stephen Smalley Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/ss/ebitmap.c | 20 ++++++++++++++++++-- security/selinux/ss/ebitmap.h | 2 +- security/selinux/ss/mls.c | 22 +++++++--------------- security/selinux/ss/mls_types.h | 2 +- 4 files changed, 27 insertions(+), 19 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 30f119b1d1ec..820313a04d49 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -213,7 +213,12 @@ netlbl_import_failure: } #endif /* CONFIG_NETLABEL */ -int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2) +/* + * Check to see if all the bits set in e2 are also set in e1. Optionally, + * if last_e2bit is non-zero, the highest set bit in e2 cannot exceed + * last_e2bit. + */ +int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit) { struct ebitmap_node *n1, *n2; int i; @@ -223,14 +228,25 @@ int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2) n1 = e1->node; n2 = e2->node; + while (n1 && n2 && (n1->startbit <= n2->startbit)) { if (n1->startbit < n2->startbit) { n1 = n1->next; continue; } - for (i = 0; i < EBITMAP_UNIT_NUMS; i++) { + for (i = EBITMAP_UNIT_NUMS - 1; (i >= 0) && !n2->maps[i]; ) + i--; /* Skip trailing NULL map entries */ + if (last_e2bit && (i >= 0)) { + u32 lastsetbit = n2->startbit + i * EBITMAP_UNIT_SIZE + + __fls(n2->maps[i]); + if (lastsetbit > last_e2bit) + return 0; + } + + while (i >= 0) { if ((n1->maps[i] & n2->maps[i]) != n2->maps[i]) return 0; + i--; } n1 = n1->next; diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 922f8afa89dd..e7eb3a9c5ab7 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -117,7 +117,7 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n, int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2); int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src); -int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2); +int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit); int ebitmap_get_bit(struct ebitmap *e, unsigned long bit); int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value); void ebitmap_destroy(struct ebitmap *e); diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index 40de8d3f208e..c85bc1ec040c 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -160,8 +160,6 @@ void mls_sid_to_context(struct context *context, int mls_level_isvalid(struct policydb *p, struct mls_level *l) { struct level_datum *levdatum; - struct ebitmap_node *node; - int i; if (!l->sens || l->sens > p->p_levels.nprim) return 0; @@ -170,19 +168,13 @@ int mls_level_isvalid(struct policydb *p, struct mls_level *l) if (!levdatum) return 0; - ebitmap_for_each_positive_bit(&l->cat, node, i) { - if (i > p->p_cats.nprim) - return 0; - if (!ebitmap_get_bit(&levdatum->level->cat, i)) { - /* - * Category may not be associated with - * sensitivity. - */ - return 0; - } - } - - return 1; + /* + * Return 1 iff all the bits set in l->cat are also be set in + * levdatum->level->cat and no bit in l->cat is larger than + * p->p_cats.nprim. + */ + return ebitmap_contains(&levdatum->level->cat, &l->cat, + p->p_cats.nprim); } int mls_range_isvalid(struct policydb *p, struct mls_range *r) diff --git a/security/selinux/ss/mls_types.h b/security/selinux/ss/mls_types.h index 03bed52a8052..e93648774137 100644 --- a/security/selinux/ss/mls_types.h +++ b/security/selinux/ss/mls_types.h @@ -35,7 +35,7 @@ static inline int mls_level_eq(struct mls_level *l1, struct mls_level *l2) static inline int mls_level_dom(struct mls_level *l1, struct mls_level *l2) { return ((l1->sens >= l2->sens) && - ebitmap_contains(&l1->cat, &l2->cat)); + ebitmap_contains(&l1->cat, &l2->cat, 0)); } #define mls_level_incomp(l1, l2) \ -- cgit v1.2.3-55-g7522 From a767f680e34bf14a36fefbbb6d85783eef99fd57 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Tue, 23 Jul 2013 17:38:41 -0400 Subject: SELinux: Increase ebitmap_node size for 64-bit configuration Currently, the ebitmap_node structure has a fixed size of 32 bytes. On a 32-bit system, the overhead is 8 bytes, leaving 24 bytes for being used as bitmaps. The overhead ratio is 1/4. On a 64-bit system, the overhead is 16 bytes. Therefore, only 16 bytes are left for bitmap purpose and the overhead ratio is 1/2. With a 3.8.2 kernel, a boot-up operation will cause the ebitmap_get_bit() function to be called about 9 million times. The average number of ebitmap_node traversal is about 3.7. This patch increases the size of the ebitmap_node structure to 64 bytes for 64-bit system to keep the overhead ratio at 1/4. This may also improve performance a little bit by making node to node traversal less frequent (< 2) as more bits are available in each node. Signed-off-by: Waiman Long Acked-by: Stephen Smalley Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/ss/ebitmap.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index e7eb3a9c5ab7..712c8a7b8e8b 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -16,7 +16,13 @@ #include -#define EBITMAP_UNIT_NUMS ((32 - sizeof(void *) - sizeof(u32)) \ +#ifdef CONFIG_64BIT +#define EBITMAP_NODE_SIZE 64 +#else +#define EBITMAP_NODE_SIZE 32 +#endif + +#define EBITMAP_UNIT_NUMS ((EBITMAP_NODE_SIZE-sizeof(void *)-sizeof(u32))\ / sizeof(unsigned long)) #define EBITMAP_UNIT_SIZE BITS_PER_LONG #define EBITMAP_SIZE (EBITMAP_UNIT_NUMS * EBITMAP_UNIT_SIZE) -- cgit v1.2.3-55-g7522 From 5c73fceb8c70466c5876ad94c356922ae75a0820 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 23 Jul 2013 17:38:41 -0400 Subject: SELinux: Enable setting security contexts on rootfs inodes. rootfs (ramfs) can support setting of security contexts by userspace due to the vfs fallback behavior of calling the security module to set the in-core inode state for security.* attributes when the filesystem does not provide an xattr handler. No xattr handler required as the inodes are pinned in memory and have no backing store. This is useful in allowing early userspace to label individual files within a rootfs while still providing a policy-defined default via genfs. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/hooks.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4bc068b3773d..911b780fcf80 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -406,6 +406,13 @@ static int sb_finish_set_opts(struct super_block *sb) if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) sbsec->flags |= SE_SBLABELSUPP; + /* + * Special handling for rootfs. Is genfs but supports + * setting SELinux context on in-core inodes. + */ + if (strncmp(sb->s_type->name, "rootfs", sizeof("rootfs")) == 0) + sbsec->flags |= SE_SBLABELSUPP; + /* Initialize the root inode. */ rc = inode_doinit_with_dentry(root_inode, root); -- cgit v1.2.3-55-g7522 From b138004ea0382bdc6d02599c39392651b4f63889 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 23 Jul 2013 17:38:42 -0400 Subject: SELinux: fix selinuxfs policy file on big endian systems The /sys/fs/selinux/policy file is not valid on big endian systems like ppc64 or s390. Let's see why: static int hashtab_cnt(void *key, void *data, void *ptr) { int *cnt = ptr; *cnt = *cnt + 1; return 0; } static int range_write(struct policydb *p, void *fp) { size_t nel; [...] /* count the number of entries in the hashtab */ nel = 0; rc = hashtab_map(p->range_tr, hashtab_cnt, &nel); if (rc) return rc; buf[0] = cpu_to_le32(nel); rc = put_entry(buf, sizeof(u32), 1, fp); So size_t is 64 bits. But then we pass a pointer to it as we do to hashtab_cnt. hashtab_cnt thinks it is a 32 bit int and only deals with the first 4 bytes. On x86_64 which is little endian, those first 4 bytes and the least significant, so this works out fine. On ppc64/s390 those first 4 bytes of memory are the high order bits. So at the end of the call to hashtab_map nel has a HUGE number. But the least significant 32 bits are all 0's. We then pass that 64 bit number to cpu_to_le32() which happily truncates it to a 32 bit number and does endian swapping. But the low 32 bits are all 0's. So no matter how many entries are in the hashtab, big endian systems always say there are 0 entries because I screwed up the counting. The fix is easy. Use a 32 bit int, as the hashtab_cnt expects, for nel. Signed-off-by: Eric Paris Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 9cd9b7c661ec..3fc8969b499c 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -3200,9 +3200,8 @@ static int range_write_helper(void *key, void *data, void *ptr) static int range_write(struct policydb *p, void *fp) { - size_t nel; __le32 buf[1]; - int rc; + int rc, nel; struct policy_data pd; pd.p = p; -- cgit v1.2.3-55-g7522 From 40d3d0b85fa22084fc3b7eeb943aca365097cea3 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:58:45 -0400 Subject: SELinux: remove crazy contortions around proc We check if the fsname is proc and if so set the proc superblock security struct flag. We then check if the flag is set and use the string 'proc' for the fsname instead of just using the fsname. What's the point? It's always proc... Get rid of the useless conditional. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 911b780fcf80..42b538ceb8b4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -678,7 +678,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->flags |= SE_SBPROC; /* Determine the labeling behavior to use for this filesystem type. */ - rc = security_fs_use((sbsec->flags & SE_SBPROC) ? "proc" : sb->s_type->name, &sbsec->behavior, &sbsec->sid); + rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid); if (rc) { printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n", __func__, sb->s_type->name, rc); -- cgit v1.2.3-55-g7522 From d355987f47bbe24e7450b509a3f8aac0db88b65a Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:58:53 -0400 Subject: SELinux: make it harder to get the number of mnt opts wrong Instead of just hard coding a value, use the enum to out benefit. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 42b538ceb8b4..863f20590186 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -94,8 +94,6 @@ #include "audit.h" #include "avc_ss.h" -#define NUM_SEL_MNT_OPTS 5 - extern struct security_operations *security_ops; /* SECMARK reference count */ @@ -307,8 +305,11 @@ enum { Opt_defcontext = 3, Opt_rootcontext = 4, Opt_labelsupport = 5, + Opt_nextmntopt = 6, }; +#define NUM_SEL_MNT_OPTS (Opt_nextmntopt - 1) + static const match_table_t tokens = { {Opt_context, CONTEXT_STR "%s"}, {Opt_fscontext, FSCONTEXT_STR "%s"}, -- cgit v1.2.3-55-g7522 From af8e50cc7d546c508e9091bbbdf3cf8b243bd8cd Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:59:00 -0400 Subject: SELinux: use define for number of bits in the mnt flags mask We had this random hard coded value of '8' in the code (I put it there) for the number of bits to check for mount options. This is stupid. Instead use the #define we already have which tells us the number of mount options. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 863f20590186..e13d65a62104 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -466,9 +466,12 @@ static int selinux_get_mnt_opts(const struct super_block *sb, if (!ss_initialized) return -EINVAL; + /* make sure we always check enough bits to cover the mask */ + BUILD_BUG_ON(SE_MNTMASK >= (1 << NUM_SEL_MNT_OPTS)); + tmp = sbsec->flags & SE_MNTMASK; /* count the number of mount options for this sb */ - for (i = 0; i < 8; i++) { + for (i = 0; i < NUM_SEL_MNT_OPTS; i++) { if (tmp & 0x01) opts->num_mnt_opts++; tmp >>= 1; -- cgit v1.2.3-55-g7522 From 12f348b9dcf6d9616c86a049c3c8700f9dc0af55 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 9 Oct 2012 10:56:25 -0400 Subject: SELinux: rename SE_SBLABELSUPP to SBLABEL_MNT Just a flag rename as we prepare to make it not so special. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 28 ++++++++++++++-------------- security/selinux/include/security.h | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e13d65a62104..344f4f999681 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -387,7 +387,7 @@ static int sb_finish_set_opts(struct super_block *sb) } } - sbsec->flags |= (SE_SBINITIALIZED | SE_SBLABELSUPP); + sbsec->flags |= (SE_SBINITIALIZED | SBLABEL_MNT); if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n", @@ -401,18 +401,18 @@ static int sb_finish_set_opts(struct super_block *sb) sbsec->behavior == SECURITY_FS_USE_MNTPOINT || sbsec->behavior == SECURITY_FS_USE_NONE || sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) - sbsec->flags &= ~SE_SBLABELSUPP; + sbsec->flags &= ~SBLABEL_MNT; /* Special handling for sysfs. Is genfs but also has setxattr handler*/ if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) - sbsec->flags |= SE_SBLABELSUPP; + sbsec->flags |= SBLABEL_MNT; /* * Special handling for rootfs. Is genfs but supports * setting SELinux context on in-core inodes. */ if (strncmp(sb->s_type->name, "rootfs", sizeof("rootfs")) == 0) - sbsec->flags |= SE_SBLABELSUPP; + sbsec->flags |= SBLABEL_MNT; /* Initialize the root inode. */ rc = inode_doinit_with_dentry(root_inode, root); @@ -477,7 +477,7 @@ static int selinux_get_mnt_opts(const struct super_block *sb, tmp >>= 1; } /* Check if the Label support flag is set */ - if (sbsec->flags & SE_SBLABELSUPP) + if (sbsec->flags & SBLABEL_MNT) opts->num_mnt_opts++; opts->mnt_opts = kcalloc(opts->num_mnt_opts, sizeof(char *), GFP_ATOMIC); @@ -524,9 +524,9 @@ static int selinux_get_mnt_opts(const struct super_block *sb, opts->mnt_opts[i] = context; opts->mnt_opts_flags[i++] = ROOTCONTEXT_MNT; } - if (sbsec->flags & SE_SBLABELSUPP) { + if (sbsec->flags & SBLABEL_MNT) { opts->mnt_opts[i] = NULL; - opts->mnt_opts_flags[i++] = SE_SBLABELSUPP; + opts->mnt_opts_flags[i++] = SBLABEL_MNT; } BUG_ON(i != opts->num_mnt_opts); @@ -615,7 +615,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, for (i = 0; i < num_opts; i++) { u32 sid; - if (flags[i] == SE_SBLABELSUPP) + if (flags[i] == SBLABEL_MNT) continue; rc = security_context_to_sid(mount_options[i], strlen(mount_options[i]), &sid); @@ -1025,7 +1025,7 @@ static void selinux_write_opts(struct seq_file *m, case DEFCONTEXT_MNT: prefix = DEFCONTEXT_STR; break; - case SE_SBLABELSUPP: + case SBLABEL_MNT: seq_putc(m, ','); seq_puts(m, LABELSUPP_STR); continue; @@ -1624,7 +1624,7 @@ static int may_create(struct inode *dir, if (rc) return rc; - if (!newsid || !(sbsec->flags & SE_SBLABELSUPP)) { + if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { rc = security_transition_sid(sid, dsec->sid, tclass, &dentry->d_name, &newsid); if (rc) @@ -2412,7 +2412,7 @@ static int selinux_sb_remount(struct super_block *sb, void *data) u32 sid; size_t len; - if (flags[i] == SE_SBLABELSUPP) + if (flags[i] == SBLABEL_MNT) continue; len = strlen(mount_options[i]); rc = security_context_to_sid(mount_options[i], len, &sid); @@ -2546,7 +2546,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, if ((sbsec->flags & SE_SBINITIALIZED) && (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) newsid = sbsec->mntpoint_sid; - else if (!newsid || !(sbsec->flags & SE_SBLABELSUPP)) { + else if (!newsid || !(sbsec->flags & SBLABEL_MNT)) { rc = security_transition_sid(sid, dsec->sid, inode_mode_to_security_class(inode->i_mode), qstr, &newsid); @@ -2568,7 +2568,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, isec->initialized = 1; } - if (!ss_initialized || !(sbsec->flags & SE_SBLABELSUPP)) + if (!ss_initialized || !(sbsec->flags & SBLABEL_MNT)) return -EOPNOTSUPP; if (name) { @@ -2776,7 +2776,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, return selinux_inode_setotherxattr(dentry, name); sbsec = inode->i_sb->s_security; - if (!(sbsec->flags & SE_SBLABELSUPP)) + if (!(sbsec->flags & SBLABEL_MNT)) return -EOPNOTSUPP; if (!inode_owner_or_capable(inode)) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 6d3885165d14..7ec750609cca 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -52,7 +52,7 @@ /* Non-mount related flags */ #define SE_SBINITIALIZED 0x10 #define SE_SBPROC 0x20 -#define SE_SBLABELSUPP 0x40 +#define SBLABEL_MNT 0x40 #define CONTEXT_STR "context=" #define FSCONTEXT_STR "fscontext=" -- cgit v1.2.3-55-g7522 From eadcabc697e904e0d93d10070a324d8855740b91 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:59:14 -0400 Subject: SELinux: do all flags twiddling in one place Currently we set the initialize and seclabel flag in one place. Do some unrelated printk then we unset the seclabel flag. Eww. Instead do the flag twiddling in one place in the code not seperated by unrelated printk. Also don't set and unset the seclabel flag. Only set it if we need to. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 344f4f999681..ca0a1c671f0e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -387,8 +387,6 @@ static int sb_finish_set_opts(struct super_block *sb) } } - sbsec->flags |= (SE_SBINITIALIZED | SBLABEL_MNT); - if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n", sb->s_id, sb->s_type->name); @@ -397,11 +395,11 @@ static int sb_finish_set_opts(struct super_block *sb) sb->s_id, sb->s_type->name, labeling_behaviors[sbsec->behavior-1]); - if (sbsec->behavior == SECURITY_FS_USE_GENFS || - sbsec->behavior == SECURITY_FS_USE_MNTPOINT || - sbsec->behavior == SECURITY_FS_USE_NONE || - sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) - sbsec->flags &= ~SBLABEL_MNT; + sbsec->flags |= SE_SBINITIALIZED; + if (sbsec->behavior == SECURITY_FS_USE_XATTR || + sbsec->behavior == SECURITY_FS_USE_TRANS || + sbsec->behavior == SECURITY_FS_USE_TASK) + sbsec->flags |= SBLABEL_MNT; /* Special handling for sysfs. Is genfs but also has setxattr handler*/ if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) -- cgit v1.2.3-55-g7522 From cfca0303da0e2c3f570cb5cfc7c373828e6c13a2 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 9 Oct 2012 16:20:08 -0400 Subject: SELinux: renumber the superblock options Just to make it clear that we have mount time options and flags, separate them. Since I decided to move the non-mount options above above 0x10, we need a short instead of a char. (x86 padding says this takes up no additional space as we have a 3byte whole in the structure) Signed-off-by: Eric Paris --- security/selinux/include/objsec.h | 2 +- security/selinux/include/security.h | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index aa47bcabb5f6..fe99f64a1d54 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -59,7 +59,7 @@ struct superblock_security_struct { u32 def_sid; /* default SID for labeling */ u32 mntpoint_sid; /* SECURITY_FS_USE_MNTPOINT context for files */ unsigned int behavior; /* labeling behavior */ - unsigned char flags; /* which mount options were specified */ + unsigned short flags; /* which mount options were specified */ struct mutex lock; struct list_head isec_head; spinlock_t isec_lock; diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 7ec750609cca..9c424d8bd2e0 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -45,14 +45,15 @@ /* Mask for just the mount related flags */ #define SE_MNTMASK 0x0f /* Super block security struct flags for mount options */ +/* BE CAREFUL, these need to be the low order bits for selinux_get_mnt_opts */ #define CONTEXT_MNT 0x01 #define FSCONTEXT_MNT 0x02 #define ROOTCONTEXT_MNT 0x04 #define DEFCONTEXT_MNT 0x08 +#define SBLABEL_MNT 0x10 /* Non-mount related flags */ -#define SE_SBINITIALIZED 0x10 -#define SE_SBPROC 0x20 -#define SBLABEL_MNT 0x40 +#define SE_SBINITIALIZED 0x0100 +#define SE_SBPROC 0x0200 #define CONTEXT_STR "context=" #define FSCONTEXT_STR "fscontext=" -- cgit v1.2.3-55-g7522 From f936c6e502d3bc21b87c9830b3a24d1e07e6b6e1 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 10 Oct 2012 10:38:47 -0400 Subject: SELinux: change sbsec->behavior to short We only have 6 options, so char is good enough, but use a short as that packs nicely. This shrinks the superblock_security_struct just a little bit. Signed-off-by: Eric Paris --- security/selinux/include/objsec.h | 2 +- security/selinux/include/security.h | 2 +- security/selinux/ss/services.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index fe99f64a1d54..b1dfe1049450 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -58,7 +58,7 @@ struct superblock_security_struct { u32 sid; /* SID of file system superblock */ u32 def_sid; /* default SID for labeling */ u32 mntpoint_sid; /* SECURITY_FS_USE_MNTPOINT context for files */ - unsigned int behavior; /* labeling behavior */ + unsigned short behavior; /* labeling behavior */ unsigned short flags; /* which mount options were specified */ struct mutex lock; struct list_head isec_head; diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 9c424d8bd2e0..20830be6b61f 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -171,7 +171,7 @@ int security_get_allow_unknown(void); #define SECURITY_FS_USE_NONE 5 /* no labeling support */ #define SECURITY_FS_USE_MNTPOINT 6 /* use mountpoint labeling */ -int security_fs_use(const char *fstype, unsigned int *behavior, +int security_fs_use(const char *fstype, short unsigned int *behavior, u32 *sid); int security_genfs_sid(const char *fstype, char *name, u16 sclass, diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index b4feecc3fe01..603c638434bb 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2329,7 +2329,7 @@ out: */ int security_fs_use( const char *fstype, - unsigned int *behavior, + short unsigned int *behavior, u32 *sid) { int rc = 0; -- cgit v1.2.3-55-g7522 From 308ab70c465d97cf7e3168961dfd365535de21a6 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:59:21 -0400 Subject: SELinux: do not handle seclabel as a special flag Instead of having special code around the 'non-mount' seclabel mount option just handle it like the mount options. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 3 --- security/selinux/include/security.h | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ca0a1c671f0e..5596dc51e21b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -474,9 +474,6 @@ static int selinux_get_mnt_opts(const struct super_block *sb, opts->num_mnt_opts++; tmp >>= 1; } - /* Check if the Label support flag is set */ - if (sbsec->flags & SBLABEL_MNT) - opts->num_mnt_opts++; opts->mnt_opts = kcalloc(opts->num_mnt_opts, sizeof(char *), GFP_ATOMIC); if (!opts->mnt_opts) { diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 20830be6b61f..1a73fcd51d56 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -43,7 +43,7 @@ #endif /* Mask for just the mount related flags */ -#define SE_MNTMASK 0x0f +#define SE_MNTMASK 0x1f /* Super block security struct flags for mount options */ /* BE CAREFUL, these need to be the low order bits for selinux_get_mnt_opts */ #define CONTEXT_MNT 0x01 -- cgit v1.2.3-55-g7522 From a64c54cf0811b8032fdab8c9d52576f0370837fa Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 24 Aug 2012 15:59:07 -0400 Subject: SELinux: pass a superblock to security_fs_use Rather than passing pointers to memory locations, strings, and other stuff just give up on the separation and give security_fs_use the superblock. It just makes the code easier to read (even if not easier to reuse on some other OS) Signed-off-by: Eric Paris --- security/selinux/hooks.c | 2 +- security/selinux/include/security.h | 3 +-- security/selinux/ss/services.c | 21 +++++++++------------ 3 files changed, 11 insertions(+), 15 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5596dc51e21b..ec15a5694b9e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -677,7 +677,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->flags |= SE_SBPROC; /* Determine the labeling behavior to use for this filesystem type. */ - rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid); + rc = security_fs_use(sb); if (rc) { printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n", __func__, sb->s_type->name, rc); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 1a73fcd51d56..01a0382c43ca 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -171,8 +171,7 @@ int security_get_allow_unknown(void); #define SECURITY_FS_USE_NONE 5 /* no labeling support */ #define SECURITY_FS_USE_MNTPOINT 6 /* use mountpoint labeling */ -int security_fs_use(const char *fstype, short unsigned int *behavior, - u32 *sid); +int security_fs_use(struct super_block *sb); int security_genfs_sid(const char *fstype, char *name, u16 sclass, u32 *sid); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 603c638434bb..a90721771615 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2323,17 +2323,14 @@ out: /** * security_fs_use - Determine how to handle labeling for a filesystem. - * @fstype: filesystem type - * @behavior: labeling behavior - * @sid: SID for filesystem (superblock) + * @sb: superblock in question */ -int security_fs_use( - const char *fstype, - short unsigned int *behavior, - u32 *sid) +int security_fs_use(struct super_block *sb) { int rc = 0; struct ocontext *c; + struct superblock_security_struct *sbsec = sb->s_security; + const char *fstype = sb->s_type->name; read_lock(&policy_rwlock); @@ -2345,21 +2342,21 @@ int security_fs_use( } if (c) { - *behavior = c->v.behavior; + sbsec->behavior = c->v.behavior; if (!c->sid[0]) { rc = sidtab_context_to_sid(&sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; } - *sid = c->sid[0]; + sbsec->sid = c->sid[0]; } else { - rc = security_genfs_sid(fstype, "/", SECCLASS_DIR, sid); + rc = security_genfs_sid(fstype, "/", SECCLASS_DIR, &sbsec->sid); if (rc) { - *behavior = SECURITY_FS_USE_NONE; + sbsec->behavior = SECURITY_FS_USE_NONE; rc = 0; } else { - *behavior = SECURITY_FS_USE_GENFS; + sbsec->behavior = SECURITY_FS_USE_GENFS; } } -- cgit v1.2.3-55-g7522 From b43e725d8d386bf2092473953b525aaae71b6c28 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 10 Oct 2012 14:27:35 -0400 Subject: SELinux: use a helper function to determine seclabel Use a helper to determine if a superblock should have the seclabel flag rather than doing it in the function. I'm going to use this in the security server as well. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ec15a5694b9e..b222e966babe 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -354,6 +354,29 @@ static int may_context_mount_inode_relabel(u32 sid, return rc; } +static int selinux_is_sblabel_mnt(struct super_block *sb) +{ + struct superblock_security_struct *sbsec = sb->s_security; + + if (sbsec->behavior == SECURITY_FS_USE_XATTR || + sbsec->behavior == SECURITY_FS_USE_TRANS || + sbsec->behavior == SECURITY_FS_USE_TASK) + return 1; + + /* Special handling for sysfs. Is genfs but also has setxattr handler*/ + if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) + return 1; + + /* + * Special handling for rootfs. Is genfs but supports + * setting SELinux context on in-core inodes. + */ + if (strncmp(sb->s_type->name, "rootfs", sizeof("rootfs")) == 0) + return 1; + + return 0; +} + static int sb_finish_set_opts(struct super_block *sb) { struct superblock_security_struct *sbsec = sb->s_security; @@ -396,20 +419,7 @@ static int sb_finish_set_opts(struct super_block *sb) labeling_behaviors[sbsec->behavior-1]); sbsec->flags |= SE_SBINITIALIZED; - if (sbsec->behavior == SECURITY_FS_USE_XATTR || - sbsec->behavior == SECURITY_FS_USE_TRANS || - sbsec->behavior == SECURITY_FS_USE_TASK) - sbsec->flags |= SBLABEL_MNT; - - /* Special handling for sysfs. Is genfs but also has setxattr handler*/ - if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) - sbsec->flags |= SBLABEL_MNT; - - /* - * Special handling for rootfs. Is genfs but supports - * setting SELinux context on in-core inodes. - */ - if (strncmp(sb->s_type->name, "rootfs", sizeof("rootfs")) == 0) + if (selinux_is_sblabel_mnt(sb)) sbsec->flags |= SBLABEL_MNT; /* Initialize the root inode. */ -- cgit v1.2.3-55-g7522 From b04eea886409de7460b5727b5931fb0bd417275f Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 23 Jul 2013 17:38:38 -0400 Subject: selinux: fix problems in netnode when BUG() is compiled out When the BUG() macro is disabled at compile time it can cause some problems in the SELinux netnode code: invalid return codes and uninitialized variables. This patch fixes this by making sure we take some corrective action after the BUG() macro. Reported-by: Geert Uytterhoeven Signed-off-by: Paul Moore Signed-off-by: Eric Paris --- security/selinux/netnode.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security/selinux') diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index c5454c0477c3..03a72c32afd7 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -166,6 +166,7 @@ static void sel_netnode_insert(struct sel_netnode *node) break; default: BUG(); + return; } /* we need to impose a limit on the growth of the hash table so check @@ -225,6 +226,7 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid) break; default: BUG(); + ret = -EINVAL; } if (ret != 0) goto out; -- cgit v1.2.3-55-g7522 From 2be4d74f2fd45460d70d4fe65cc1972ef45bf849 Mon Sep 17 00:00:00 2001 From: Chris PeBenito Date: Fri, 3 May 2013 09:05:39 -0400 Subject: Add SELinux policy capability for always checking packet and peer classes. Currently the packet class in SELinux is not checked if there are no SECMARK rules in the security or mangle netfilter tables. Some systems prefer that packets are always checked, for example, to protect the system should the netfilter rules fail to load or if the nefilter rules were maliciously flushed. Add the always_check_network policy capability which, when enabled, treats SECMARK as enabled, even if there are no netfilter SECMARK rules and treats peer labeling as enabled, even if there is no Netlabel or labeled IPSEC configuration. Includes definition of "redhat1" SELinux policy capability, which exists in the SELinux userpace library, to keep ordering correct. The SELinux userpace portion of this was merged last year, but this kernel change fell on the floor. Signed-off-by: Chris PeBenito Signed-off-by: Eric Paris --- security/selinux/hooks.c | 26 +++++++++++++++++++++----- security/selinux/include/security.h | 3 +++ security/selinux/selinuxfs.c | 4 +++- security/selinux/ss/services.c | 3 +++ 4 files changed, 30 insertions(+), 6 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b222e966babe..4fbf2c5f26ce 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -136,12 +136,28 @@ static struct kmem_cache *sel_inode_cache; * This function checks the SECMARK reference counter to see if any SECMARK * targets are currently configured, if the reference counter is greater than * zero SECMARK is considered to be enabled. Returns true (1) if SECMARK is - * enabled, false (0) if SECMARK is disabled. + * enabled, false (0) if SECMARK is disabled. If the always_check_network + * policy capability is enabled, SECMARK is always considered enabled. * */ static int selinux_secmark_enabled(void) { - return (atomic_read(&selinux_secmark_refcount) > 0); + return (selinux_policycap_alwaysnetwork || atomic_read(&selinux_secmark_refcount)); +} + +/** + * selinux_peerlbl_enabled - Check to see if peer labeling is currently enabled + * + * Description: + * This function checks if NetLabel or labeled IPSEC is enabled. Returns true + * (1) if any are enabled or false (0) if neither are enabled. If the + * always_check_network policy capability is enabled, peer labeling + * is always considered enabled. + * + */ +static int selinux_peerlbl_enabled(void) +{ + return (selinux_policycap_alwaysnetwork || netlbl_enabled() || selinux_xfrm_enabled()); } /* @@ -4197,7 +4213,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) return selinux_sock_rcv_skb_compat(sk, skb, family); secmark_active = selinux_secmark_enabled(); - peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled(); + peerlbl_active = selinux_peerlbl_enabled(); if (!secmark_active && !peerlbl_active) return 0; @@ -4579,7 +4595,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb, int ifindex, secmark_active = selinux_secmark_enabled(); netlbl_active = netlbl_enabled(); - peerlbl_active = netlbl_active || selinux_xfrm_enabled(); + peerlbl_active = selinux_peerlbl_enabled(); if (!secmark_active && !peerlbl_active) return NF_ACCEPT; @@ -4731,7 +4747,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex, return NF_ACCEPT; #endif secmark_active = selinux_secmark_enabled(); - peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled(); + peerlbl_active = selinux_peerlbl_enabled(); if (!secmark_active && !peerlbl_active) return NF_ACCEPT; diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 01a0382c43ca..004a2479880f 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -69,12 +69,15 @@ extern int selinux_enabled; enum { POLICYDB_CAPABILITY_NETPEER, POLICYDB_CAPABILITY_OPENPERM, + POLICYDB_CAPABILITY_REDHAT1, + POLICYDB_CAPABILITY_ALWAYSNETWORK, __POLICYDB_CAPABILITY_MAX }; #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) extern int selinux_policycap_netpeer; extern int selinux_policycap_openperm; +extern int selinux_policycap_alwaysnetwork; /* * type_datum properties diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index ff427733c290..5122affe06a8 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -44,7 +44,9 @@ /* Policy capability filenames */ static char *policycap_names[] = { "network_peer_controls", - "open_perms" + "open_perms", + "redhat1", + "always_check_network" }; unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a90721771615..d106733ad987 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -72,6 +72,7 @@ int selinux_policycap_netpeer; int selinux_policycap_openperm; +int selinux_policycap_alwaysnetwork; static DEFINE_RWLOCK(policy_rwlock); @@ -1812,6 +1813,8 @@ static void security_load_policycaps(void) POLICYDB_CAPABILITY_NETPEER); selinux_policycap_openperm = ebitmap_get_bit(&policydb.policycaps, POLICYDB_CAPABILITY_OPENPERM); + selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps, + POLICYDB_CAPABILITY_ALWAYSNETWORK); } static int security_preserve_bools(struct policydb *p); -- cgit v1.2.3-55-g7522 From 102aefdda4d8275ce7d7100bc16c88c74272b260 Mon Sep 17 00:00:00 2001 From: Anand Avati Date: Tue, 16 Apr 2013 18:56:19 -0400 Subject: selinux: consider filesystem subtype in policies Not considering sub filesystem has the following limitation. Support for SELinux in FUSE is dependent on the particular userspace filesystem, which is identified by the subtype. For e.g, GlusterFS, a FUSE based filesystem supports SELinux (by mounting and processing FUSE requests in different threads, avoiding the mount time deadlock), whereas other FUSE based filesystems (identified by a different subtype) have the mount time deadlock. By considering the subtype of the filesytem in the SELinux policies, allows us to specify a filesystem subtype, in the following way: fs_use_xattr fuse.glusterfs gen_context(system_u:object_r:fs_t,s0); This way not all FUSE filesystems are put in the same bucket and subjected to the limitations of the other subtypes. Signed-off-by: Anand Avati Signed-off-by: Eric Paris --- security/selinux/hooks.c | 40 ++++++++++++++++++++++------------------ security/selinux/ss/services.c | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 22 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4fbf2c5f26ce..0d4408debb45 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -94,6 +94,10 @@ #include "audit.h" #include "avc_ss.h" +#define SB_TYPE_FMT "%s%s%s" +#define SB_SUBTYPE(sb) (sb->s_subtype && sb->s_subtype[0]) +#define SB_TYPE_ARGS(sb) sb->s_type->name, SB_SUBTYPE(sb) ? "." : "", SB_SUBTYPE(sb) ? sb->s_subtype : "" + extern struct security_operations *security_ops; /* SECMARK reference count */ @@ -407,8 +411,8 @@ static int sb_finish_set_opts(struct super_block *sb) the first boot of the SELinux kernel before we have assigned xattr values to the filesystem. */ if (!root_inode->i_op->getxattr) { - printk(KERN_WARNING "SELinux: (dev %s, type %s) has no " - "xattr support\n", sb->s_id, sb->s_type->name); + printk(KERN_WARNING "SELinux: (dev %s, type "SB_TYPE_FMT") has no " + "xattr support\n", sb->s_id, SB_TYPE_ARGS(sb)); rc = -EOPNOTSUPP; goto out; } @@ -416,22 +420,22 @@ static int sb_finish_set_opts(struct super_block *sb) if (rc < 0 && rc != -ENODATA) { if (rc == -EOPNOTSUPP) printk(KERN_WARNING "SELinux: (dev %s, type " - "%s) has no security xattr handler\n", - sb->s_id, sb->s_type->name); + SB_TYPE_FMT") has no security xattr handler\n", + sb->s_id, SB_TYPE_ARGS(sb)); else printk(KERN_WARNING "SELinux: (dev %s, type " - "%s) getxattr errno %d\n", sb->s_id, - sb->s_type->name, -rc); + SB_TYPE_FMT") getxattr errno %d\n", sb->s_id, + SB_TYPE_ARGS(sb), -rc); goto out; } } if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) - printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n", - sb->s_id, sb->s_type->name); + printk(KERN_ERR "SELinux: initialized (dev %s, type "SB_TYPE_FMT"), unknown behavior\n", + sb->s_id, SB_TYPE_ARGS(sb)); else - printk(KERN_DEBUG "SELinux: initialized (dev %s, type %s), %s\n", - sb->s_id, sb->s_type->name, + printk(KERN_DEBUG "SELinux: initialized (dev %s, type "SB_TYPE_FMT"), %s\n", + sb->s_id, SB_TYPE_ARGS(sb), labeling_behaviors[sbsec->behavior-1]); sbsec->flags |= SE_SBINITIALIZED; @@ -589,7 +593,6 @@ static int selinux_set_mnt_opts(struct super_block *sb, const struct cred *cred = current_cred(); int rc = 0, i; struct superblock_security_struct *sbsec = sb->s_security; - const char *name = sb->s_type->name; struct inode *inode = sbsec->sb->s_root->d_inode; struct inode_security_struct *root_isec = inode->i_security; u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0; @@ -642,8 +645,8 @@ static int selinux_set_mnt_opts(struct super_block *sb, strlen(mount_options[i]), &sid); if (rc) { printk(KERN_WARNING "SELinux: security_context_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - mount_options[i], sb->s_id, name, rc); + "(%s) failed for (dev %s, type "SB_TYPE_FMT") errno=%d\n", + mount_options[i], sb->s_id, SB_TYPE_ARGS(sb), rc); goto out; } switch (flags[i]) { @@ -779,7 +782,8 @@ out: out_double_mount: rc = -EINVAL; printk(KERN_WARNING "SELinux: mount invalid. Same superblock, different " - "security settings for (dev %s, type %s)\n", sb->s_id, name); + "security settings for (dev %s, type "SB_TYPE_FMT")\n", sb->s_id, + SB_TYPE_ARGS(sb)); goto out; } @@ -2439,8 +2443,8 @@ static int selinux_sb_remount(struct super_block *sb, void *data) rc = security_context_to_sid(mount_options[i], len, &sid); if (rc) { printk(KERN_WARNING "SELinux: security_context_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - mount_options[i], sb->s_id, sb->s_type->name, rc); + "(%s) failed for (dev %s, type "SB_TYPE_FMT") errno=%d\n", + mount_options[i], sb->s_id, SB_TYPE_ARGS(sb), rc); goto out_free_opts; } rc = -EINVAL; @@ -2478,8 +2482,8 @@ out_free_secdata: return rc; out_bad_option: printk(KERN_WARNING "SELinux: unable to change security options " - "during remount (dev %s, type=%s)\n", sb->s_id, - sb->s_type->name); + "during remount (dev %s, type "SB_TYPE_FMT")\n", sb->s_id, + SB_TYPE_ARGS(sb)); goto out_free_opts; } diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index d106733ad987..ee470a0b5c27 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2334,16 +2334,50 @@ int security_fs_use(struct super_block *sb) struct ocontext *c; struct superblock_security_struct *sbsec = sb->s_security; const char *fstype = sb->s_type->name; + const char *subtype = (sb->s_subtype && sb->s_subtype[0]) ? sb->s_subtype : NULL; + struct ocontext *base = NULL; read_lock(&policy_rwlock); - c = policydb.ocontexts[OCON_FSUSE]; - while (c) { - if (strcmp(fstype, c->u.name) == 0) + for (c = policydb.ocontexts[OCON_FSUSE]; c; c = c->next) { + char *sub; + int baselen; + + baselen = strlen(fstype); + + /* if base does not match, this is not the one */ + if (strncmp(fstype, c->u.name, baselen)) + continue; + + /* if there is no subtype, this is the one! */ + if (!subtype) + break; + + /* skip past the base in this entry */ + sub = c->u.name + baselen; + + /* entry is only a base. save it. keep looking for subtype */ + if (sub[0] == '\0') { + base = c; + continue; + } + + /* entry is not followed by a subtype, so it is not a match */ + if (sub[0] != '.') + continue; + + /* whew, we found a subtype of this fstype */ + sub++; /* move past '.' */ + + /* exact match of fstype AND subtype */ + if (!strcmp(subtype, sub)) break; - c = c->next; } + /* in case we had found an fstype match but no subtype match */ + if (!c) + c = base; + if (c) { sbsec->behavior = c->v.behavior; if (!c->sid[0]) { -- cgit v1.2.3-55-g7522 From 0b4bdb3573a86a88c829b9e4ad702859eb923e7e Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 28 Aug 2013 13:32:42 -0400 Subject: Revert "SELinux: do not handle seclabel as a special flag" This reverts commit 308ab70c465d97cf7e3168961dfd365535de21a6. It breaks my FC6 test box. /dev/pts is not mounted. dmesg says SELinux: mount invalid. Same superblock, different security settings for (dev devpts, type devpts) Cc: Peter Hurley Cc: Greg KH Signed-off-by: Andrew Morton Signed-off-by: Eric Paris --- security/selinux/hooks.c | 3 +++ security/selinux/include/security.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0d4408debb45..c156f5eb1aea 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -504,6 +504,9 @@ static int selinux_get_mnt_opts(const struct super_block *sb, opts->num_mnt_opts++; tmp >>= 1; } + /* Check if the Label support flag is set */ + if (sbsec->flags & SBLABEL_MNT) + opts->num_mnt_opts++; opts->mnt_opts = kcalloc(opts->num_mnt_opts, sizeof(char *), GFP_ATOMIC); if (!opts->mnt_opts) { diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 004a2479880f..7aad3a1389d1 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -43,7 +43,7 @@ #endif /* Mask for just the mount related flags */ -#define SE_MNTMASK 0x1f +#define SE_MNTMASK 0x0f /* Super block security struct flags for mount options */ /* BE CAREFUL, these need to be the low order bits for selinux_get_mnt_opts */ #define CONTEXT_MNT 0x01 -- cgit v1.2.3-55-g7522 From 7d1db4b242134bb4c7170f4c15491980ac064ac3 Mon Sep 17 00:00:00 2001 From: Duan Jiong Date: Thu, 26 Sep 2013 15:52:13 -0400 Subject: selinux: Use kmemdup instead of kmalloc + memcpy Signed-off-by: Duan Jiong Signed-off-by: Paul Moore --- security/selinux/xfrm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 425b9f91d755..a91d205ec0c6 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -268,10 +268,10 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, if (!old_ctx) return 0; - new_ctx = kmalloc(sizeof(*old_ctx) + old_ctx->ctx_len, GFP_ATOMIC); + new_ctx = kmemdup(old_ctx, sizeof(*old_ctx) + old_ctx->ctx_len, + GFP_ATOMIC); if (!new_ctx) return -ENOMEM; - memcpy(new_ctx, old_ctx, sizeof(*old_ctx) + old_ctx->ctx_len); atomic_inc(&selinux_xfrm_refcount); *new_ctxp = new_ctx; -- cgit v1.2.3-55-g7522 From 42d64e1add3a1ce8a787116036163b8724362145 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 26 Sep 2013 17:00:46 -0400 Subject: selinux: correct locking in selinux_netlbl_socket_connect) The SELinux/NetLabel glue code has a locking bug that affects systems with NetLabel enabled, see the kernel error message below. This patch corrects this problem by converting the bottom half socket lock to a more conventional, and correct for this call-path, lock_sock() call. =============================== [ INFO: suspicious RCU usage. ] 3.11.0-rc3+ #19 Not tainted ------------------------------- net/ipv4/cipso_ipv4.c:1928 suspicious rcu_dereference_protected() usage! other info that might help us debug this: rcu_scheduler_active = 1, debug_locks = 0 2 locks held by ping/731: #0: (slock-AF_INET/1){+.-...}, at: [...] selinux_netlbl_socket_connect #1: (rcu_read_lock){.+.+..}, at: [<...>] netlbl_conn_setattr stack backtrace: CPU: 1 PID: 731 Comm: ping Not tainted 3.11.0-rc3+ #19 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 0000000000000001 ffff88006f659d28 ffffffff81726b6a ffff88003732c500 ffff88006f659d58 ffffffff810e4457 ffff88006b845a00 0000000000000000 000000000000000c ffff880075aa2f50 ffff88006f659d90 ffffffff8169bec7 Call Trace: [] dump_stack+0x54/0x74 [] lockdep_rcu_suspicious+0xe7/0x120 [] cipso_v4_sock_setattr+0x187/0x1a0 [] netlbl_conn_setattr+0x187/0x190 [] ? netlbl_conn_setattr+0x5/0x190 [] selinux_netlbl_socket_connect+0xae/0xc0 [] selinux_socket_connect+0x135/0x170 [] ? might_fault+0x57/0xb0 [] security_socket_connect+0x16/0x20 [] SYSC_connect+0x73/0x130 [] ? sysret_check+0x22/0x5d [] ? trace_hardirqs_on_caller+0xfd/0x1c0 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] SyS_connect+0xe/0x10 [] system_call_fastpath+0x16/0x1b Cc: stable@vger.kernel.org Signed-off-by: Paul Moore --- security/selinux/netlabel.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c index da4b8b233280..6235d052338b 100644 --- a/security/selinux/netlabel.c +++ b/security/selinux/netlabel.c @@ -442,8 +442,7 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr) sksec->nlbl_state != NLBL_CONNLABELED) return 0; - local_bh_disable(); - bh_lock_sock_nested(sk); + lock_sock(sk); /* connected sockets are allowed to disconnect when the address family * is set to AF_UNSPEC, if that is what is happening we want to reset @@ -464,7 +463,6 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr) sksec->nlbl_state = NLBL_CONNLABELED; socket_connect_return: - bh_unlock_sock(sk); - local_bh_enable(); + release_sock(sk); return rc; } -- cgit v1.2.3-55-g7522