From: James Carter <jwcart2@tycho.nsa.gov>
To: Stephen Smalley <sds@tycho.nsa.gov>, selinux@tycho.nsa.gov
Subject: Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
Date: Thu, 18 Jun 2015 16:16:56 -0400 [thread overview]
Message-ID: <55832738.60606@tycho.nsa.gov> (raw)
In-Reply-To: <5582CA84.5090503@tycho.nsa.gov>
On 06/18/2015 09:41 AM, Stephen Smalley wrote:
> On 06/17/2015 03:58 PM, James Carter wrote:
>> Types are treated as attributes that contain only themselves. This
>> is how types are already treated in the type_attr_map.
>>
>> Treating types this way makes finding rules that apply to a given
>> type much easier.
>
> Can you provide an example of the difference it makes?
>
I'll add something like the following.
This allows the following statement:
ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
which would have previously been written as:
if (p->type_val_to_struct[src-1]->flavor == TYPE_ATTRIB &&
p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
} else if (p->type_val_to_struct[k->source_type - 1]->flavor == TYPE_ATTRIB) {
if (ebitmap_get_bit(&p->attr_type_map[src-1], tgt-1)) {
ebitmap_set_bit(&m, tgt-1, 1);
}
} else if (p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
if (ebitmap_get_bit(&p->attr_type_map[tgt-1], src-1)) {
ebitmap_set_bit(&m, src-1, 1);
}
} else {
if (src == tgt) {
ebitmap_set_bit(&m, src-1, 1);
}
}
>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>> ---
>> libsepol/src/expand.c | 24 ++++++++++++++++--------
>> libsepol/src/policydb.c | 4 ++++
>> 2 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>> index 478eaff..85fbe0f 100644
>> --- a/libsepol/src/expand.c
>> +++ b/libsepol/src/expand.c
>> @@ -2311,25 +2311,33 @@ static int type_attr_map(hashtab_key_t key
>> policydb_t *p = state->out;
>> unsigned int i;
>> ebitmap_node_t *tnode;
>> + int value;
>>
>> type = (type_datum_t *) datum;
>> + value = type->s.value;
>> +
>> if (type->flavor == TYPE_ATTRIB) {
>> - if (ebitmap_cpy(&p->attr_type_map[type->s.value - 1],
>> - &type->types)) {
>> - ERR(state->handle, "Out of memory!");
>> - return -1;
>> + if (ebitmap_cpy(&p->attr_type_map[value - 1], &type->types)) {
>> + goto out;
>> }
>> ebitmap_for_each_bit(&type->types, tnode, i) {
>> if (!ebitmap_node_get_bit(tnode, i))
>> continue;
>> - if (ebitmap_set_bit(&p->type_attr_map[i],
>> - type->s.value - 1, 1)) {
>> - ERR(state->handle, "Out of memory!");
>> - return -1;
>> + if (ebitmap_set_bit(&p->type_attr_map[i], value - 1, 1)) {
>> + goto out;
>> }
>> }
>> + } else {
>> + if (ebitmap_set_bit(&p->attr_type_map[value - 1], value - 1, 1)) {
>> + goto out;
>> + }
>> }
>> +
>> return 0;
>> +
>> +out:
>
> Should probably call this err: or oom: instead to avoid confusion if
> anyone later introduces an out: label for successful return path.
>
Sure, oom seems to be used a decent amount in other places.
>> + ERR(state->handle, "Out of memory!");
>> + return -1;
>> }
>>
>> /* converts typeset using typemap and expands into ebitmap_t types using the attributes in the passed in policy.
>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> index 1677eb5..670aef8 100644
>> --- a/libsepol/src/policydb.c
>> +++ b/libsepol/src/policydb.c
>> @@ -3936,6 +3936,10 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
>> /* add the type itself as the degenerate case */
>> if (ebitmap_set_bit(&p->type_attr_map[i], i, 1))
>> goto bad;
>> + if (p->type_val_to_struct[i]->flavor != TYPE_ATTRIB) {
>> + if (ebitmap_set_bit(&p->attr_type_map[i], i, 1))
>> + goto bad;
>> + }
>> }
>> }
>
> It appears to me that this won't break any existing users of
> attr_type_map AFAICS because they all check flavor == TYPE_ATTRIB first.
>
That is my view as well.
> I was wondering though if we are being inconsistent with type_attr_map.
> We do set the type itself here upon policydb_read() of an existing
> kernel policy, but do we do it when first generating the type_attr_map
> by libsepol?
>
>
I only see the type_attr_map being generated in policydb_read for a kernel
policy and when expand_module() is called. This surprised me and made me think
that there must be a better place to generate these mappings, but I can't think
of a better place. Can you?
--
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency
next prev parent reply other threads:[~2015-06-18 20:16 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
2015-06-18 13:23 ` Stephen Smalley
2015-06-17 19:58 ` [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map James Carter
2015-06-18 13:41 ` Stephen Smalley
2015-06-18 13:52 ` Stephen Smalley
2015-06-18 20:16 ` James Carter [this message]
2015-06-18 20:21 ` Stephen Smalley
2015-06-18 20:23 ` Stephen Smalley
2015-06-17 19:58 ` [PATCH 03/10 v2] libsepol: Refactored neverallow checking James Carter
2015-06-17 19:58 ` [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code James Carter
2015-06-18 13:56 ` Stephen Smalley
2015-06-18 20:26 ` James Carter
2015-06-18 20:29 ` Stephen Smalley
2015-06-18 20:35 ` James Carter
2015-06-17 19:58 ` [PATCH 05/10 v2] libsepol/cil: Add function to search the CIL AST for an AV rule James Carter
2015-06-17 19:58 ` [PATCH 06/10 v2] libsepol/cil: Refactored CIL neverallow checking and reporting James Carter
2015-06-17 19:58 ` [PATCH 07/10 v2] libsepol/cil: Track number of classes and number of types and attributes James Carter
2015-06-17 19:58 ` [PATCH 08/10 v2] libsepol/cil: Add CIL bounds checking and reporting James Carter
2015-06-17 19:58 ` [PATCH 09/10 v2] secilc: Add a CIL policy file to test neverallow checking James Carter
2015-06-17 19:58 ` [PATCH 10/10 v2] secilc: Add a CIL policy file to test bounds checking James Carter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55832738.60606@tycho.nsa.gov \
--to=jwcart2@tycho.nsa.gov \
--cc=sds@tycho.nsa.gov \
--cc=selinux@tycho.nsa.gov \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.