| Message ID | 05c7cbda-4dc0-0eb0-02bf-5a93d4a4ecb4@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/13/2016 04:36 PM, Casey Schaufler wrote: > Subject: [PATCH 14/25] SELinux: Maintain secid to secctx mapping > > Maintain the mapping from secid to secctx in context structures > that are on the sidtab. This has two important impacts: > > - Because the secctx is kept with the secid it is > not necessary to regenerate the context when looking > for the secctx by secid or the secid by secctx. > - With the secctx being kept around the interfaces > that use it don't have to make copies. > > The minor increase in memory usage (a secctx string with each > secid) is a small price to pay for the elimination of countless > alloc/free pairs in the networking and audit code. > > A new field has been added to the context structure to contain > the context. The existing "str" field is not overloaded as there > is some complexity in the way it is managed. NAK on this one; as I said before, it makes no sense to add another (len, str) pair to the context structure when we already have one there. Just add a bool field to indicate whether it is mapped/unmapped, and reuse the (str, len) pair. Similarly, sidtab_find duplicates sidtab_search and isn't necessary at all. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > security/selinux/avc.c | 8 ++----- > security/selinux/hooks.c | 49 +++++++++++++++++++----------------------- > security/selinux/selinuxfs.c | 7 ------ > security/selinux/ss/context.h | 2 ++ > security/selinux/ss/services.c | 28 ++++++++++++++---------- > security/selinux/ss/services.h | 4 ++++ > security/selinux/ss/sidtab.c | 38 ++++++++++++++++++++++++++++++++ > security/selinux/ss/sidtab.h | 1 + > security/selinux/xfrm.c | 8 ++----- > 9 files changed, 88 insertions(+), 57 deletions(-) > > diff --git a/security/selinux/avc.c b/security/selinux/avc.c > index e60c79d..09977a9 100644 > --- a/security/selinux/avc.c > +++ b/security/selinux/avc.c > @@ -152,18 +152,14 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla > rc = security_sid_to_context(ssid, &scontext, &scontext_len); > if (rc) > audit_log_format(ab, "ssid=%d", ssid); > - else { > + else > audit_log_format(ab, "scontext=%s", scontext); > - kfree(scontext); > - } > > rc = security_sid_to_context(tsid, &scontext, &scontext_len); > if (rc) > audit_log_format(ab, " tsid=%d", tsid); > - else { > + else > audit_log_format(ab, " tcontext=%s", scontext); > - kfree(scontext); > - } > > BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map)); > audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name); > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 03717c2..e8c6c5d 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2872,7 +2872,9 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > rc = security_sid_to_context_force(newsid, &context, &clen); > if (rc) > return rc; > - *value = context; > + *value = kstrdup(context, GFP_KERNEL); > + if (*value == NULL) > + return -ENOMEM; > *len = clen; > } > > @@ -3231,12 +3233,13 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void > if (error) > return error; > error = size; > - if (alloc) { > + > + /* > + * context will not be freed because selinux_release_context > + * does not free anything. > + */ > + if (alloc) > *buffer = context; > - goto out_nofree; > - } > - kfree(context); > -out_nofree: > return error; > } > > @@ -4621,7 +4624,6 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op > out_len: > if (put_user(scontext_len, optlen)) > err = -EFAULT; > - kfree(scontext); > return err; > } > > @@ -5674,6 +5676,7 @@ static int selinux_getprocattr(struct task_struct *p, > u32 sid; > int error; > unsigned len; > + char *vp; > > if (current != p) { > error = current_has_perm(p, PROCESS__GETATTR); > @@ -5705,21 +5708,17 @@ static int selinux_getprocattr(struct task_struct *p, > if (!sid) > return 0; > > - if (strcmp(name, "context")) { > - error = security_sid_to_context(sid, value, &len); > - } else { > - char *vp; > - > - error = security_sid_to_context(sid, &vp, &len); > - if (!error) { > - *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); > - if (*value == NULL) > - error = -ENOMEM; > - } > - } > - > + error = security_sid_to_context(sid, &vp, &len); > if (error) > return error; > + > + if (strcmp(name, "context")) > + *value = kstrdup(vp, GFP_KERNEL); > + else > + *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); > + > + if (*value == NULL) > + return -ENOMEM; > return len; > > invalid: > @@ -5876,11 +5875,6 @@ static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) > return security_context_to_sid(secdata, seclen, secid, GFP_KERNEL); > } > > -static void selinux_release_secctx(char *secdata, u32 seclen) > -{ > - kfree(secdata); > -} > - > static void selinux_inode_invalidate_secctx(struct inode *inode) > { > struct inode_security_struct *isec = inode->i_security; > @@ -5978,7 +5972,9 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) > rc = security_sid_to_context(ksec->sid, &context, &len); > if (!rc) > rc = len; > - *_buffer = context; > + *_buffer = kstrdup(context, GFP_KERNEL); > + if (*_buffer == NULL) > + rc = -ENOMEM; > return rc; > } > > @@ -6123,7 +6119,6 @@ static struct security_hook_list selinux_hooks[] = { > LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel), > LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), > LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), > - LSM_HOOK_INIT(release_secctx, selinux_release_secctx), > LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), > LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), > LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > index 2519e26..96674d8 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -582,7 +582,6 @@ static ssize_t sel_write_context(struct file *file, char *buf, size_t size) > memcpy(buf, canon, len); > length = len; > out: > - kfree(canon); > return length; > } > > @@ -902,7 +901,6 @@ static ssize_t sel_write_create(struct file *file, char *buf, size_t size) > memcpy(buf, newcon, len); > length = len; > out: > - kfree(newcon); > kfree(namebuf); > kfree(tcon); > kfree(scon); > @@ -959,7 +957,6 @@ static ssize_t sel_write_relabel(struct file *file, char *buf, size_t size) > memcpy(buf, newcon, len); > length = len; > out: > - kfree(newcon); > kfree(tcon); > kfree(scon); > return length; > @@ -1009,12 +1006,10 @@ static ssize_t sel_write_user(struct file *file, char *buf, size_t size) > goto out; > } > if ((length + len) >= SIMPLE_TRANSACTION_LIMIT) { > - kfree(newcon); > length = -ERANGE; > goto out; > } > memcpy(ptr, newcon, len); > - kfree(newcon); > ptr += len; > length += len; > } > @@ -1078,7 +1073,6 @@ static ssize_t sel_write_member(struct file *file, char *buf, size_t size) > memcpy(buf, newcon, len); > length = len; > out: > - kfree(newcon); > kfree(tcon); > kfree(scon); > return length; > @@ -1521,7 +1515,6 @@ static ssize_t sel_read_initcon(struct file *file, char __user *buf, > return ret; > > ret = simple_read_from_buffer(buf, count, ppos, con, len); > - kfree(con); > return ret; > } > > diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h > index 212e347..730c886 100644 > --- a/security/selinux/ss/context.h > +++ b/security/selinux/ss/context.h > @@ -30,6 +30,8 @@ struct context { > u32 len; /* length of string in bytes */ > struct mls_range range; > char *str; /* string representation if context cannot be mapped. */ > + u32 seclen; > + char *secctx; /* string representation if context can be mapped. */ > }; > > static inline void mls_context_init(struct context *c) > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 082b20c..feab3d5 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -89,8 +89,6 @@ int ss_initialized; > static u32 latest_granting; > > /* Forward declaration. */ > -static int context_struct_to_string(struct context *context, char **scontext, > - u32 *scontext_len); > > static void context_struct_compute_av(struct context *scontext, > struct context *tcontext, > @@ -1176,7 +1174,8 @@ allow: > * to point to this string and set `*scontext_len' to > * the length of the string. > */ > -static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len) > +int context_struct_to_string(struct context *context, char **scontext, > + u32 *scontext_len) > { > char *scontextp; > > @@ -1236,6 +1235,7 @@ const char *security_get_initial_sid_context(u32 sid) > static int security_sid_to_context_core(u32 sid, char **scontext, > u32 *scontext_len, int force) > { > + struct sidtab_node *snode; > struct context *context; > int rc = 0; > > @@ -1245,18 +1245,11 @@ static int security_sid_to_context_core(u32 sid, char **scontext, > > if (!ss_initialized) { > if (sid <= SECINITSID_NUM) { > - char *scontextp; > > *scontext_len = strlen(initial_sid_to_string[sid]) + 1; > if (!scontext) > goto out; > - scontextp = kmemdup(initial_sid_to_string[sid], > - *scontext_len, GFP_ATOMIC); > - if (!scontextp) { > - rc = -ENOMEM; > - goto out; > - } > - *scontext = scontextp; > + *scontext = (char *)initial_sid_to_string[sid]; > goto out; > } > printk(KERN_ERR "SELinux: %s: called before initial " > @@ -1265,6 +1258,19 @@ static int security_sid_to_context_core(u32 sid, char **scontext, > goto out; > } > read_lock(&policy_rwlock); > + snode = sidtab_find(&sidtab, sid, force); > + if (!snode) { > + printk(KERN_ERR "SELinux: %s: unrecognized SID tab %d\n", > + __func__, sid); > + } else if (snode->context.secctx == NULL || > + snode->context.seclen == 0) { > + printk(KERN_ERR "SELinux: %s: unrecognized SID secctx %d\n", > + __func__, sid); > + } else { > + *scontext = snode->context.secctx; > + *scontext_len = snode->context.seclen; > + goto out_unlock; > + } > if (force) > context = sidtab_search_force(&sidtab, sid); > else > diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h > index 6abcd87..dd9dffd 100644 > --- a/security/selinux/ss/services.h > +++ b/security/selinux/ss/services.h > @@ -17,5 +17,9 @@ void services_compute_xperms_drivers(struct extended_perms *xperms, > void services_compute_xperms_decision(struct extended_perms_decision *xpermd, > struct avtab_node *node); > > +int context_struct_to_string(struct context *context, char **scontext, > + u32 *scontext_len); > + > + > #endif /* _SS_SERVICES_H_ */ > > diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c > index 5840a35..e18c38f 100644 > --- a/security/selinux/ss/sidtab.c > +++ b/security/selinux/ss/sidtab.c > @@ -9,6 +9,7 @@ > #include <linux/errno.h> > #include "flask.h" > #include "security.h" > +#include "services.h" > #include "sidtab.h" > > #define SIDTAB_HASH(sid) \ > @@ -64,6 +65,13 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) > rc = -ENOMEM; > goto out; > } > + if (context_struct_to_string(context, &newnode->context.secctx, > + &newnode->context.seclen) < 0) { > + context_destroy(&newnode->context); > + kfree(newnode); > + rc = -ENOMEM; > + goto out; > + } > > if (prev) { > newnode->next = prev->next; > @@ -82,6 +90,36 @@ out: > return rc; > } > > +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force) > +{ > + int hvalue; > + struct sidtab_node *cur; > + > + if (!s) > + return NULL; > + > + hvalue = SIDTAB_HASH(sid); > + cur = s->htable[hvalue]; > + while (cur && sid > cur->sid) > + cur = cur->next; > + > + if (force && cur && sid == cur->sid && cur->context.len) > + return cur; > + > + if (cur == NULL || sid != cur->sid || cur->context.len) { > + /* Remap invalid SIDs to the unlabeled SID. */ > + sid = SECINITSID_UNLABELED; > + hvalue = SIDTAB_HASH(sid); > + cur = s->htable[hvalue]; > + while (cur && sid > cur->sid) > + cur = cur->next; > + if (!cur || sid != cur->sid) > + return NULL; > + } > + > + return cur; > +} > + > static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) > { > int hvalue; > diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h > index 84dc154..0d60b380 100644 > --- a/security/selinux/ss/sidtab.h > +++ b/security/selinux/ss/sidtab.h > @@ -35,6 +35,7 @@ int sidtab_init(struct sidtab *s); > int sidtab_insert(struct sidtab *s, u32 sid, struct context *context); > struct context *sidtab_search(struct sidtab *s, u32 sid); > struct context *sidtab_search_force(struct sidtab *s, u32 sid); > +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force); > > int sidtab_map(struct sidtab *s, > int (*apply) (u32 sid, > diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c > index 789d07b..ca165d3 100644 > --- a/security/selinux/xfrm.c > +++ b/security/selinux/xfrm.c > @@ -357,10 +357,8 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, > return rc; > > ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC); > - if (!ctx) { > - rc = -ENOMEM; > - goto out; > - } > + if (!ctx) > + return -ENOMEM; > > ctx->ctx_doi = XFRM_SC_DOI_LSM; > ctx->ctx_alg = XFRM_SC_ALG_SELINUX; > @@ -370,8 +368,6 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, > > x->security = ctx; > atomic_inc(&selinux_xfrm_refcount); > -out: > - kfree(ctx_str); > return rc; > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/15/2016 5:20 AM, Stephen Smalley wrote: > On 08/13/2016 04:36 PM, Casey Schaufler wrote: >> Subject: [PATCH 14/25] SELinux: Maintain secid to secctx mapping >> >> Maintain the mapping from secid to secctx in context structures >> that are on the sidtab. This has two important impacts: >> >> - Because the secctx is kept with the secid it is >> not necessary to regenerate the context when looking >> for the secctx by secid or the secid by secctx. >> - With the secctx being kept around the interfaces >> that use it don't have to make copies. >> >> The minor increase in memory usage (a secctx string with each >> secid) is a small price to pay for the elimination of countless >> alloc/free pairs in the networking and audit code. >> >> A new field has been added to the context structure to contain >> the context. The existing "str" field is not overloaded as there >> is some complexity in the way it is managed. > NAK on this one; as I said before, it makes no sense to add another > (len, str) pair to the context structure What you NAKed before was adding them to the sidtab. > when we already have one there. > Just add a bool field to indicate whether it is mapped/unmapped, and > reuse the (str, len) pair. That was the other choice. I personally prefer the separate pointer, but will make the change. The details of how (len, str) are used has me a little confused, but I'll work it out. > > Similarly, sidtab_find duplicates sidtab_search and isn't necessary at all. That's one of the places where the details of the use of (len, str) had me a little baffled. Not to worry. I will make the change. >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> security/selinux/avc.c | 8 ++----- >> security/selinux/hooks.c | 49 +++++++++++++++++++----------------------- >> security/selinux/selinuxfs.c | 7 ------ >> security/selinux/ss/context.h | 2 ++ >> security/selinux/ss/services.c | 28 ++++++++++++++---------- >> security/selinux/ss/services.h | 4 ++++ >> security/selinux/ss/sidtab.c | 38 ++++++++++++++++++++++++++++++++ >> security/selinux/ss/sidtab.h | 1 + >> security/selinux/xfrm.c | 8 ++----- >> 9 files changed, 88 insertions(+), 57 deletions(-) >> >> diff --git a/security/selinux/avc.c b/security/selinux/avc.c >> index e60c79d..09977a9 100644 >> --- a/security/selinux/avc.c >> +++ b/security/selinux/avc.c >> @@ -152,18 +152,14 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla >> rc = security_sid_to_context(ssid, &scontext, &scontext_len); >> if (rc) >> audit_log_format(ab, "ssid=%d", ssid); >> - else { >> + else >> audit_log_format(ab, "scontext=%s", scontext); >> - kfree(scontext); >> - } >> >> rc = security_sid_to_context(tsid, &scontext, &scontext_len); >> if (rc) >> audit_log_format(ab, " tsid=%d", tsid); >> - else { >> + else >> audit_log_format(ab, " tcontext=%s", scontext); >> - kfree(scontext); >> - } >> >> BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map)); >> audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name); >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 03717c2..e8c6c5d 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -2872,7 +2872,9 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, >> rc = security_sid_to_context_force(newsid, &context, &clen); >> if (rc) >> return rc; >> - *value = context; >> + *value = kstrdup(context, GFP_KERNEL); >> + if (*value == NULL) >> + return -ENOMEM; >> *len = clen; >> } >> >> @@ -3231,12 +3233,13 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void >> if (error) >> return error; >> error = size; >> - if (alloc) { >> + >> + /* >> + * context will not be freed because selinux_release_context >> + * does not free anything. >> + */ >> + if (alloc) >> *buffer = context; >> - goto out_nofree; >> - } >> - kfree(context); >> -out_nofree: >> return error; >> } >> >> @@ -4621,7 +4624,6 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op >> out_len: >> if (put_user(scontext_len, optlen)) >> err = -EFAULT; >> - kfree(scontext); >> return err; >> } >> >> @@ -5674,6 +5676,7 @@ static int selinux_getprocattr(struct task_struct *p, >> u32 sid; >> int error; >> unsigned len; >> + char *vp; >> >> if (current != p) { >> error = current_has_perm(p, PROCESS__GETATTR); >> @@ -5705,21 +5708,17 @@ static int selinux_getprocattr(struct task_struct *p, >> if (!sid) >> return 0; >> >> - if (strcmp(name, "context")) { >> - error = security_sid_to_context(sid, value, &len); >> - } else { >> - char *vp; >> - >> - error = security_sid_to_context(sid, &vp, &len); >> - if (!error) { >> - *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); >> - if (*value == NULL) >> - error = -ENOMEM; >> - } >> - } >> - >> + error = security_sid_to_context(sid, &vp, &len); >> if (error) >> return error; >> + >> + if (strcmp(name, "context")) >> + *value = kstrdup(vp, GFP_KERNEL); >> + else >> + *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); >> + >> + if (*value == NULL) >> + return -ENOMEM; >> return len; >> >> invalid: >> @@ -5876,11 +5875,6 @@ static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) >> return security_context_to_sid(secdata, seclen, secid, GFP_KERNEL); >> } >> >> -static void selinux_release_secctx(char *secdata, u32 seclen) >> -{ >> - kfree(secdata); >> -} >> - >> static void selinux_inode_invalidate_secctx(struct inode *inode) >> { >> struct inode_security_struct *isec = inode->i_security; >> @@ -5978,7 +5972,9 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) >> rc = security_sid_to_context(ksec->sid, &context, &len); >> if (!rc) >> rc = len; >> - *_buffer = context; >> + *_buffer = kstrdup(context, GFP_KERNEL); >> + if (*_buffer == NULL) >> + rc = -ENOMEM; >> return rc; >> } >> >> @@ -6123,7 +6119,6 @@ static struct security_hook_list selinux_hooks[] = { >> LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel), >> LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), >> LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), >> - LSM_HOOK_INIT(release_secctx, selinux_release_secctx), >> LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), >> LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), >> LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), >> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c >> index 2519e26..96674d8 100644 >> --- a/security/selinux/selinuxfs.c >> +++ b/security/selinux/selinuxfs.c >> @@ -582,7 +582,6 @@ static ssize_t sel_write_context(struct file *file, char *buf, size_t size) >> memcpy(buf, canon, len); >> length = len; >> out: >> - kfree(canon); >> return length; >> } >> >> @@ -902,7 +901,6 @@ static ssize_t sel_write_create(struct file *file, char *buf, size_t size) >> memcpy(buf, newcon, len); >> length = len; >> out: >> - kfree(newcon); >> kfree(namebuf); >> kfree(tcon); >> kfree(scon); >> @@ -959,7 +957,6 @@ static ssize_t sel_write_relabel(struct file *file, char *buf, size_t size) >> memcpy(buf, newcon, len); >> length = len; >> out: >> - kfree(newcon); >> kfree(tcon); >> kfree(scon); >> return length; >> @@ -1009,12 +1006,10 @@ static ssize_t sel_write_user(struct file *file, char *buf, size_t size) >> goto out; >> } >> if ((length + len) >= SIMPLE_TRANSACTION_LIMIT) { >> - kfree(newcon); >> length = -ERANGE; >> goto out; >> } >> memcpy(ptr, newcon, len); >> - kfree(newcon); >> ptr += len; >> length += len; >> } >> @@ -1078,7 +1073,6 @@ static ssize_t sel_write_member(struct file *file, char *buf, size_t size) >> memcpy(buf, newcon, len); >> length = len; >> out: >> - kfree(newcon); >> kfree(tcon); >> kfree(scon); >> return length; >> @@ -1521,7 +1515,6 @@ static ssize_t sel_read_initcon(struct file *file, char __user *buf, >> return ret; >> >> ret = simple_read_from_buffer(buf, count, ppos, con, len); >> - kfree(con); >> return ret; >> } >> >> diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h >> index 212e347..730c886 100644 >> --- a/security/selinux/ss/context.h >> +++ b/security/selinux/ss/context.h >> @@ -30,6 +30,8 @@ struct context { >> u32 len; /* length of string in bytes */ >> struct mls_range range; >> char *str; /* string representation if context cannot be mapped. */ >> + u32 seclen; >> + char *secctx; /* string representation if context can be mapped. */ >> }; >> >> static inline void mls_context_init(struct context *c) >> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c >> index 082b20c..feab3d5 100644 >> --- a/security/selinux/ss/services.c >> +++ b/security/selinux/ss/services.c >> @@ -89,8 +89,6 @@ int ss_initialized; >> static u32 latest_granting; >> >> /* Forward declaration. */ >> -static int context_struct_to_string(struct context *context, char **scontext, >> - u32 *scontext_len); >> >> static void context_struct_compute_av(struct context *scontext, >> struct context *tcontext, >> @@ -1176,7 +1174,8 @@ allow: >> * to point to this string and set `*scontext_len' to >> * the length of the string. >> */ >> -static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len) >> +int context_struct_to_string(struct context *context, char **scontext, >> + u32 *scontext_len) >> { >> char *scontextp; >> >> @@ -1236,6 +1235,7 @@ const char *security_get_initial_sid_context(u32 sid) >> static int security_sid_to_context_core(u32 sid, char **scontext, >> u32 *scontext_len, int force) >> { >> + struct sidtab_node *snode; >> struct context *context; >> int rc = 0; >> >> @@ -1245,18 +1245,11 @@ static int security_sid_to_context_core(u32 sid, char **scontext, >> >> if (!ss_initialized) { >> if (sid <= SECINITSID_NUM) { >> - char *scontextp; >> >> *scontext_len = strlen(initial_sid_to_string[sid]) + 1; >> if (!scontext) >> goto out; >> - scontextp = kmemdup(initial_sid_to_string[sid], >> - *scontext_len, GFP_ATOMIC); >> - if (!scontextp) { >> - rc = -ENOMEM; >> - goto out; >> - } >> - *scontext = scontextp; >> + *scontext = (char *)initial_sid_to_string[sid]; >> goto out; >> } >> printk(KERN_ERR "SELinux: %s: called before initial " >> @@ -1265,6 +1258,19 @@ static int security_sid_to_context_core(u32 sid, char **scontext, >> goto out; >> } >> read_lock(&policy_rwlock); >> + snode = sidtab_find(&sidtab, sid, force); >> + if (!snode) { >> + printk(KERN_ERR "SELinux: %s: unrecognized SID tab %d\n", >> + __func__, sid); >> + } else if (snode->context.secctx == NULL || >> + snode->context.seclen == 0) { >> + printk(KERN_ERR "SELinux: %s: unrecognized SID secctx %d\n", >> + __func__, sid); >> + } else { >> + *scontext = snode->context.secctx; >> + *scontext_len = snode->context.seclen; >> + goto out_unlock; >> + } >> if (force) >> context = sidtab_search_force(&sidtab, sid); >> else >> diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h >> index 6abcd87..dd9dffd 100644 >> --- a/security/selinux/ss/services.h >> +++ b/security/selinux/ss/services.h >> @@ -17,5 +17,9 @@ void services_compute_xperms_drivers(struct extended_perms *xperms, >> void services_compute_xperms_decision(struct extended_perms_decision *xpermd, >> struct avtab_node *node); >> >> +int context_struct_to_string(struct context *context, char **scontext, >> + u32 *scontext_len); >> + >> + >> #endif /* _SS_SERVICES_H_ */ >> >> diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c >> index 5840a35..e18c38f 100644 >> --- a/security/selinux/ss/sidtab.c >> +++ b/security/selinux/ss/sidtab.c >> @@ -9,6 +9,7 @@ >> #include <linux/errno.h> >> #include "flask.h" >> #include "security.h" >> +#include "services.h" >> #include "sidtab.h" >> >> #define SIDTAB_HASH(sid) \ >> @@ -64,6 +65,13 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) >> rc = -ENOMEM; >> goto out; >> } >> + if (context_struct_to_string(context, &newnode->context.secctx, >> + &newnode->context.seclen) < 0) { >> + context_destroy(&newnode->context); >> + kfree(newnode); >> + rc = -ENOMEM; >> + goto out; >> + } >> >> if (prev) { >> newnode->next = prev->next; >> @@ -82,6 +90,36 @@ out: >> return rc; >> } >> >> +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force) >> +{ >> + int hvalue; >> + struct sidtab_node *cur; >> + >> + if (!s) >> + return NULL; >> + >> + hvalue = SIDTAB_HASH(sid); >> + cur = s->htable[hvalue]; >> + while (cur && sid > cur->sid) >> + cur = cur->next; >> + >> + if (force && cur && sid == cur->sid && cur->context.len) >> + return cur; >> + >> + if (cur == NULL || sid != cur->sid || cur->context.len) { >> + /* Remap invalid SIDs to the unlabeled SID. */ >> + sid = SECINITSID_UNLABELED; >> + hvalue = SIDTAB_HASH(sid); >> + cur = s->htable[hvalue]; >> + while (cur && sid > cur->sid) >> + cur = cur->next; >> + if (!cur || sid != cur->sid) >> + return NULL; >> + } >> + >> + return cur; >> +} >> + >> static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) >> { >> int hvalue; >> diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h >> index 84dc154..0d60b380 100644 >> --- a/security/selinux/ss/sidtab.h >> +++ b/security/selinux/ss/sidtab.h >> @@ -35,6 +35,7 @@ int sidtab_init(struct sidtab *s); >> int sidtab_insert(struct sidtab *s, u32 sid, struct context *context); >> struct context *sidtab_search(struct sidtab *s, u32 sid); >> struct context *sidtab_search_force(struct sidtab *s, u32 sid); >> +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force); >> >> int sidtab_map(struct sidtab *s, >> int (*apply) (u32 sid, >> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c >> index 789d07b..ca165d3 100644 >> --- a/security/selinux/xfrm.c >> +++ b/security/selinux/xfrm.c >> @@ -357,10 +357,8 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, >> return rc; >> >> ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC); >> - if (!ctx) { >> - rc = -ENOMEM; >> - goto out; >> - } >> + if (!ctx) >> + return -ENOMEM; >> >> ctx->ctx_doi = XFRM_SC_DOI_LSM; >> ctx->ctx_alg = XFRM_SC_ALG_SELINUX; >> @@ -370,8 +368,6 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, >> >> x->security = ctx; >> atomic_inc(&selinux_xfrm_refcount); >> -out: >> - kfree(ctx_str); >> return rc; >> } >> >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/selinux/avc.c b/security/selinux/avc.c index e60c79d..09977a9 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -152,18 +152,14 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla rc = security_sid_to_context(ssid, &scontext, &scontext_len); if (rc) audit_log_format(ab, "ssid=%d", ssid); - else { + else audit_log_format(ab, "scontext=%s", scontext); - kfree(scontext); - } rc = security_sid_to_context(tsid, &scontext, &scontext_len); if (rc) audit_log_format(ab, " tsid=%d", tsid); - else { + else audit_log_format(ab, " tcontext=%s", scontext); - kfree(scontext); - } BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map)); audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 03717c2..e8c6c5d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2872,7 +2872,9 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, rc = security_sid_to_context_force(newsid, &context, &clen); if (rc) return rc; - *value = context; + *value = kstrdup(context, GFP_KERNEL); + if (*value == NULL) + return -ENOMEM; *len = clen; } @@ -3231,12 +3233,13 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void if (error) return error; error = size; - if (alloc) { + + /* + * context will not be freed because selinux_release_context + * does not free anything. + */ + if (alloc) *buffer = context; - goto out_nofree; - } - kfree(context); -out_nofree: return error; } @@ -4621,7 +4624,6 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op out_len: if (put_user(scontext_len, optlen)) err = -EFAULT; - kfree(scontext); return err; } @@ -5674,6 +5676,7 @@ static int selinux_getprocattr(struct task_struct *p, u32 sid; int error; unsigned len; + char *vp; if (current != p) { error = current_has_perm(p, PROCESS__GETATTR); @@ -5705,21 +5708,17 @@ static int selinux_getprocattr(struct task_struct *p, if (!sid) return 0; - if (strcmp(name, "context")) { - error = security_sid_to_context(sid, value, &len); - } else { - char *vp; - - error = security_sid_to_context(sid, &vp, &len); - if (!error) { - *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); - if (*value == NULL) - error = -ENOMEM; - } - } - + error = security_sid_to_context(sid, &vp, &len); if (error) return error; + + if (strcmp(name, "context")) + *value = kstrdup(vp, GFP_KERNEL); + else + *value = kasprintf(GFP_KERNEL, "selinux='%s'", vp); + + if (*value == NULL) + return -ENOMEM; return len; invalid: @@ -5876,11 +5875,6 @@ static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) return security_context_to_sid(secdata, seclen, secid, GFP_KERNEL); } -static void selinux_release_secctx(char *secdata, u32 seclen) -{ - kfree(secdata); -} - static void selinux_inode_invalidate_secctx(struct inode *inode) { struct inode_security_struct *isec = inode->i_security; @@ -5978,7 +5972,9 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) rc = security_sid_to_context(ksec->sid, &context, &len); if (!rc) rc = len; - *_buffer = context; + *_buffer = kstrdup(context, GFP_KERNEL); + if (*_buffer == NULL) + rc = -ENOMEM; return rc; } @@ -6123,7 +6119,6 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel), LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), - LSM_HOOK_INIT(release_secctx, selinux_release_secctx), LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 2519e26..96674d8 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -582,7 +582,6 @@ static ssize_t sel_write_context(struct file *file, char *buf, size_t size) memcpy(buf, canon, len); length = len; out: - kfree(canon); return length; } @@ -902,7 +901,6 @@ static ssize_t sel_write_create(struct file *file, char *buf, size_t size) memcpy(buf, newcon, len); length = len; out: - kfree(newcon); kfree(namebuf); kfree(tcon); kfree(scon); @@ -959,7 +957,6 @@ static ssize_t sel_write_relabel(struct file *file, char *buf, size_t size) memcpy(buf, newcon, len); length = len; out: - kfree(newcon); kfree(tcon); kfree(scon); return length; @@ -1009,12 +1006,10 @@ static ssize_t sel_write_user(struct file *file, char *buf, size_t size) goto out; } if ((length + len) >= SIMPLE_TRANSACTION_LIMIT) { - kfree(newcon); length = -ERANGE; goto out; } memcpy(ptr, newcon, len); - kfree(newcon); ptr += len; length += len; } @@ -1078,7 +1073,6 @@ static ssize_t sel_write_member(struct file *file, char *buf, size_t size) memcpy(buf, newcon, len); length = len; out: - kfree(newcon); kfree(tcon); kfree(scon); return length; @@ -1521,7 +1515,6 @@ static ssize_t sel_read_initcon(struct file *file, char __user *buf, return ret; ret = simple_read_from_buffer(buf, count, ppos, con, len); - kfree(con); return ret; } diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 212e347..730c886 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -30,6 +30,8 @@ struct context { u32 len; /* length of string in bytes */ struct mls_range range; char *str; /* string representation if context cannot be mapped. */ + u32 seclen; + char *secctx; /* string representation if context can be mapped. */ }; static inline void mls_context_init(struct context *c) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 082b20c..feab3d5 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -89,8 +89,6 @@ int ss_initialized; static u32 latest_granting; /* Forward declaration. */ -static int context_struct_to_string(struct context *context, char **scontext, - u32 *scontext_len); static void context_struct_compute_av(struct context *scontext, struct context *tcontext, @@ -1176,7 +1174,8 @@ allow: * to point to this string and set `*scontext_len' to * the length of the string. */ -static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len) +int context_struct_to_string(struct context *context, char **scontext, + u32 *scontext_len) { char *scontextp; @@ -1236,6 +1235,7 @@ const char *security_get_initial_sid_context(u32 sid) static int security_sid_to_context_core(u32 sid, char **scontext, u32 *scontext_len, int force) { + struct sidtab_node *snode; struct context *context; int rc = 0; @@ -1245,18 +1245,11 @@ static int security_sid_to_context_core(u32 sid, char **scontext, if (!ss_initialized) { if (sid <= SECINITSID_NUM) { - char *scontextp; *scontext_len = strlen(initial_sid_to_string[sid]) + 1; if (!scontext) goto out; - scontextp = kmemdup(initial_sid_to_string[sid], - *scontext_len, GFP_ATOMIC); - if (!scontextp) { - rc = -ENOMEM; - goto out; - } - *scontext = scontextp; + *scontext = (char *)initial_sid_to_string[sid]; goto out; } printk(KERN_ERR "SELinux: %s: called before initial " @@ -1265,6 +1258,19 @@ static int security_sid_to_context_core(u32 sid, char **scontext, goto out; } read_lock(&policy_rwlock); + snode = sidtab_find(&sidtab, sid, force); + if (!snode) { + printk(KERN_ERR "SELinux: %s: unrecognized SID tab %d\n", + __func__, sid); + } else if (snode->context.secctx == NULL || + snode->context.seclen == 0) { + printk(KERN_ERR "SELinux: %s: unrecognized SID secctx %d\n", + __func__, sid); + } else { + *scontext = snode->context.secctx; + *scontext_len = snode->context.seclen; + goto out_unlock; + } if (force) context = sidtab_search_force(&sidtab, sid); else diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index 6abcd87..dd9dffd 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -17,5 +17,9 @@ void services_compute_xperms_drivers(struct extended_perms *xperms, void services_compute_xperms_decision(struct extended_perms_decision *xpermd, struct avtab_node *node); +int context_struct_to_string(struct context *context, char **scontext, + u32 *scontext_len); + + #endif /* _SS_SERVICES_H_ */ diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 5840a35..e18c38f 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -9,6 +9,7 @@ #include <linux/errno.h> #include "flask.h" #include "security.h" +#include "services.h" #include "sidtab.h" #define SIDTAB_HASH(sid) \ @@ -64,6 +65,13 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) rc = -ENOMEM; goto out; } + if (context_struct_to_string(context, &newnode->context.secctx, + &newnode->context.seclen) < 0) { + context_destroy(&newnode->context); + kfree(newnode); + rc = -ENOMEM; + goto out; + } if (prev) { newnode->next = prev->next; @@ -82,6 +90,36 @@ out: return rc; } +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force) +{ + int hvalue; + struct sidtab_node *cur; + + if (!s) + return NULL; + + hvalue = SIDTAB_HASH(sid); + cur = s->htable[hvalue]; + while (cur && sid > cur->sid) + cur = cur->next; + + if (force && cur && sid == cur->sid && cur->context.len) + return cur; + + if (cur == NULL || sid != cur->sid || cur->context.len) { + /* Remap invalid SIDs to the unlabeled SID. */ + sid = SECINITSID_UNLABELED; + hvalue = SIDTAB_HASH(sid); + cur = s->htable[hvalue]; + while (cur && sid > cur->sid) + cur = cur->next; + if (!cur || sid != cur->sid) + return NULL; + } + + return cur; +} + static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) { int hvalue; diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h index 84dc154..0d60b380 100644 --- a/security/selinux/ss/sidtab.h +++ b/security/selinux/ss/sidtab.h @@ -35,6 +35,7 @@ int sidtab_init(struct sidtab *s); int sidtab_insert(struct sidtab *s, u32 sid, struct context *context); struct context *sidtab_search(struct sidtab *s, u32 sid); struct context *sidtab_search_force(struct sidtab *s, u32 sid); +struct sidtab_node *sidtab_find(struct sidtab *s, u32 sid, int force); int sidtab_map(struct sidtab *s, int (*apply) (u32 sid, diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 789d07b..ca165d3 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -357,10 +357,8 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, return rc; ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC); - if (!ctx) { - rc = -ENOMEM; - goto out; - } + if (!ctx) + return -ENOMEM; ctx->ctx_doi = XFRM_SC_DOI_LSM; ctx->ctx_alg = XFRM_SC_ALG_SELINUX; @@ -370,8 +368,6 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, x->security = ctx; atomic_inc(&selinux_xfrm_refcount); -out: - kfree(ctx_str); return rc; }
Subject: [PATCH 14/25] SELinux: Maintain secid to secctx mapping Maintain the mapping from secid to secctx in context structures that are on the sidtab. This has two important impacts: - Because the secctx is kept with the secid it is not necessary to regenerate the context when looking for the secctx by secid or the secid by secctx. - With the secctx being kept around the interfaces that use it don't have to make copies. The minor increase in memory usage (a secctx string with each secid) is a small price to pay for the elimination of countless alloc/free pairs in the networking and audit code. A new field has been added to the context structure to contain the context. The existing "str" field is not overloaded as there is some complexity in the way it is managed. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- security/selinux/avc.c | 8 ++----- security/selinux/hooks.c | 49 +++++++++++++++++++----------------------- security/selinux/selinuxfs.c | 7 ------ security/selinux/ss/context.h | 2 ++ security/selinux/ss/services.c | 28 ++++++++++++++---------- security/selinux/ss/services.h | 4 ++++ security/selinux/ss/sidtab.c | 38 ++++++++++++++++++++++++++++++++ security/selinux/ss/sidtab.h | 1 + security/selinux/xfrm.c | 8 ++----- 9 files changed, 88 insertions(+), 57 deletions(-)