On 04/13/2011 11:23 AM, Steve Lawrence wrote: > Thanks for the patch. This solves the problem, but I'm still seeing > issues that seems related to support of older version of policy when > using role_transition with non-process classes. I'm now seeing an mls > range overflow error: > > libsepol.role_trans_write: Discarding role_transition rules for security > class other than "process" > libsepol.mls_read_range_helper: range overflow > libsepol.context_read_and_validate: error reading MLS range of context > libsepol.policydb_to_image: new policy image is invalid > > I'm still looking into it, but I'm not too familiar this part of the code. > > > On 04/12/2011 05:11 PM, Eric Paris wrote: >> Although the role trans code had support to handle the kernel policy >> when the version was less that roletrans such support was not in the >> module read/write code. This patch adds proper support for role trans >> in modules. >> >> Signed-off-by: Eric Paris <eparis@xxxxxxxxxx> >> --- >> libsepol/src/policydb.c | 14 ++++++++++---- >> libsepol/src/write.c | 37 ++++++++++++++++++++++++++++++++----- >> 2 files changed, 42 insertions(+), 9 deletions(-) >> >> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c >> index 2ecb636..6d8ff91 100644 >> --- a/libsepol/src/policydb.c >> +++ b/libsepol/src/policydb.c >> @@ -3033,7 +3033,8 @@ int avrule_read_list(policydb_t * p, avrule_t ** avrules, >> return 0; >> } >> >> -static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp) >> +static int role_trans_rule_read(policydb_t *p, role_trans_rule_t ** r, >> + struct policy_file *fp) >> { >> uint32_t buf[1], nel; >> unsigned int i; >> @@ -3064,8 +3065,13 @@ static int role_trans_rule_read(role_trans_rule_t ** r, struct policy_file *fp) >> if (type_set_read(&tr->types, fp)) >> return -1; >> >> - if (ebitmap_read(&tr->classes, fp)) >> - return -1; >> + if (p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS) { >> + if (ebitmap_read(&tr->classes, fp)) >> + return -1; >> + } else { >> + if (ebitmap_set_bit(&tr->classes, SECCLASS_PROCESS - 1, 1)) >> + return -1; >> + } >> >> rc = next_entry(buf, fp, sizeof(uint32_t)); >> if (rc < 0) >> @@ -3258,7 +3264,7 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl, >> decl->enabled = le32_to_cpu(buf[1]); >> if (cond_read_list(p, &decl->cond_list, fp) == -1 || >> avrule_read_list(p, &decl->avrules, fp) == -1 || >> - role_trans_rule_read(&decl->role_tr_rules, fp) == -1 || >> + role_trans_rule_read(p, &decl->role_tr_rules, fp) == -1 || >> role_allow_rule_read(&decl->role_allow_rules, fp) == -1) { >> return -1; >> } >> diff --git a/libsepol/src/write.c b/libsepol/src/write.c >> index c4f5035..78b3aa6 100644 >> --- a/libsepol/src/write.c >> +++ b/libsepol/src/write.c >> @@ -1482,26 +1482,53 @@ static int avrule_write_list(avrule_t * avrules, struct policy_file *fp) >> return POLICYDB_SUCCESS; >> } >> >> -static int role_trans_rule_write(role_trans_rule_t * t, struct policy_file *fp) >> +static int only_process(ebitmap_t *in) >> +{ >> + unsigned int i; >> + ebitmap_node_t *node; >> + >> + ebitmap_for_each_bit(in, node, i) { >> + if (ebitmap_node_get_bit(node, i) && >> + i != SECCLASS_PROCESS - 1) >> + return 0; >> + } >> + return 1; >> +} >> + >> +static int role_trans_rule_write(policydb_t *p, role_trans_rule_t * t, >> + struct policy_file *fp) >> { >> int nel = 0; >> size_t items; >> uint32_t buf[1]; >> role_trans_rule_t *tr; >> + int warned = 0; >> + int new_role = p->policyvers >= MOD_POLICYDB_VERSION_ROLETRANS; >> >> for (tr = t; tr; tr = tr->next) >> - nel++; >> + if (new_role || only_process(&tr->classes)) >> + nel++; >> + >> buf[0] = cpu_to_le32(nel); >> items = put_entry(buf, sizeof(uint32_t), 1, fp); >> if (items != 1) >> return POLICYDB_ERROR; >> for (tr = t; tr; tr = tr->next) { >> + if (!new_role && !only_process(&tr->classes)) { >> + if (!warned) >> + WARN(fp->handle, "Discarding role_transition " >> + "rules for security classes other than " >> + "\"process\""); >> + warned = 1; >> + continue; >> + } >> if (role_set_write(&tr->roles, fp)) >> return POLICYDB_ERROR; >> if (type_set_write(&tr->types, fp)) >> return POLICYDB_ERROR; >> - if (ebitmap_write(&tr->classes, fp)) >> - return POLICYDB_ERROR; >> + if (new_role) >> + if (ebitmap_write(&tr->classes, fp)) >> + return POLICYDB_ERROR; >> buf[0] = cpu_to_le32(tr->new_role); >> items = put_entry(buf, sizeof(uint32_t), 1, fp); >> if (items != 1) >> @@ -1636,7 +1663,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms, >> } >> if (cond_write_list(p, decl->cond_list, fp) == -1 || >> avrule_write_list(decl->avrules, fp) == -1 || >> - role_trans_rule_write(decl->role_tr_rules, fp) == -1 || >> + role_trans_rule_write(p, decl->role_tr_rules, fp) == -1 || >> role_allow_rule_write(decl->role_allow_rules, fp) == -1) { >> return POLICYDB_ERROR; >> } > > > -- > This message was distributed to subscribers of the selinux mailing list. > If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with > the words "unsubscribe selinux" without quotes as the message. I think the below patch fixes the problem I was seeing. Some rules were getting discarded in role_trans_write during a policy downgrade, but the count was not being decreased. This fix is similar to your fix in role_trans_rule_write. --- diff --git a/libsepol/src/write.c b/libsepol/src/write.c index 78b3aa6..3a9c35a 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -474,7 +474,9 @@ static int role_trans_write(policydb_t *p, struct policy_file *fp) nel = 0; for (tr = r; tr; tr = tr->next) - nel++; + if(new_roletr || tr->class == SECCLASS_PROCESS) + nel++; + buf[0] = cpu_to_le32(nel); items = put_entry(buf, sizeof(uint32_t), 1, fp); if (items != 1) -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.