On Mon, Aug 8, 2022 at 11:01 AM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Thu, Jul 21, 2022 at 11:11 AM Christian Göttsche
> <cgzones@xxxxxxxxxxxxxx> wrote:
> >
> > Refactor the ebitmap conversions in link.c into its own function.
> >
> > Do not log an OOM message twice on type_set_or_convert() failure.
> >
> > Drop the now unused state parameter from type_set_or_convert() and
> > type_set_convert().
> >
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
>
> Acked-by: James Carter <jwcart2@xxxxxxxxx>
>
I applied this patch, but none of the others from this series.
This patch really fit better with the other ebitmap work that was
being merged anyway.
Thanks,
Jim
> > ---
> > libsepol/src/link.c | 140 +++++++++++++++-----------------------------
> > 1 file changed, 47 insertions(+), 93 deletions(-)
> >
> > diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> > index 7e8313cb..cbe4cea4 100644
> > --- a/libsepol/src/link.c
> > +++ b/libsepol/src/link.c
> > @@ -958,26 +958,28 @@ static int alias_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
> >
> > /*********** callbacks that fix bitmaps ***********/
> >
> > -static int type_set_convert(type_set_t * types, type_set_t * dst,
> > - policy_module_t * mod, link_state_t * state
> > - __attribute__ ((unused)))
> > +static int ebitmap_convert(const ebitmap_t *src, ebitmap_t *dst, const uint32_t *map)
> > {
> > - unsigned int i;
> > - ebitmap_node_t *tnode;
> > - ebitmap_for_each_positive_bit(&types->types, tnode, i) {
> > - assert(mod->map[SYM_TYPES][i]);
> > - if (ebitmap_set_bit
> > - (&dst->types, mod->map[SYM_TYPES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > - ebitmap_for_each_positive_bit(&types->negset, tnode, i) {
> > - assert(mod->map[SYM_TYPES][i]);
> > - if (ebitmap_set_bit
> > - (&dst->negset, mod->map[SYM_TYPES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > + unsigned int bit;
> > + ebitmap_node_t *node;
> > + ebitmap_for_each_positive_bit(src, node, bit) {
> > + assert(map[bit]);
> > + if (ebitmap_set_bit(dst, map[bit] - 1, 1))
> > + return -1;
> > }
> > +
> > + return 0;
> > +}
> > +
> > +static int type_set_convert(const type_set_t * types, type_set_t * dst,
> > + const policy_module_t * mod)
> > +{
> > + if (ebitmap_convert(&types->types, &dst->types, mod->map[SYM_TYPES]))
> > + goto cleanup;
> > +
> > + if (ebitmap_convert(&types->negset, &dst->negset, mod->map[SYM_TYPES]))
> > + goto cleanup;
> > +
> > dst->flags = types->flags;
> > return 0;
> >
> > @@ -988,13 +990,13 @@ static int type_set_convert(type_set_t * types, type_set_t * dst,
> > /* OR 2 typemaps together and at the same time map the src types to
> > * the correct values in the dst typeset.
> > */
> > -static int type_set_or_convert(type_set_t * types, type_set_t * dst,
> > - policy_module_t * mod, link_state_t * state)
> > +static int type_set_or_convert(const type_set_t * types, type_set_t * dst,
> > + const policy_module_t * mod)
> > {
> > type_set_t ts_tmp;
> >
> > type_set_init(&ts_tmp);
> > - if (type_set_convert(types, &ts_tmp, mod, state) == -1) {
> > + if (type_set_convert(types, &ts_tmp, mod) == -1) {
> > goto cleanup;
> > }
> > if (type_set_or_eq(dst, &ts_tmp)) {
> > @@ -1004,7 +1006,6 @@ static int type_set_or_convert(type_set_t * types, type_set_t * dst,
> > return 0;
> >
> > cleanup:
> > - ERR(state->handle, "Out of memory!");
> > type_set_destroy(&ts_tmp);
> > return -1;
> > }
> > @@ -1012,18 +1013,11 @@ static int type_set_or_convert(type_set_t * types, type_set_t * dst,
> > static int role_set_or_convert(role_set_t * roles, role_set_t * dst,
> > policy_module_t * mod, link_state_t * state)
> > {
> > - unsigned int i;
> > ebitmap_t tmp;
> > - ebitmap_node_t *rnode;
> >
> > ebitmap_init(&tmp);
> > - ebitmap_for_each_positive_bit(&roles->roles, rnode, i) {
> > - assert(mod->map[SYM_ROLES][i]);
> > - if (ebitmap_set_bit
> > - (&tmp, mod->map[SYM_ROLES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&roles->roles, &tmp, mod->map[SYM_ROLES]))
> > + goto cleanup;
> > if (ebitmap_union(&dst->roles, &tmp)) {
> > goto cleanup;
> > }
> > @@ -1088,13 +1082,11 @@ static int mls_range_convert(mls_semantic_range_t * src, mls_semantic_range_t *
> > static int role_fix_callback(hashtab_key_t key, hashtab_datum_t datum,
> > void *data)
> > {
> > - unsigned int i;
> > char *id = key;
> > role_datum_t *role, *dest_role = NULL;
> > link_state_t *state = (link_state_t *) data;
> > ebitmap_t e_tmp;
> > policy_module_t *mod = state->cur;
> > - ebitmap_node_t *rnode;
> > hashtab_t role_tab;
> >
> > role = (role_datum_t *) datum;
> > @@ -1111,30 +1103,20 @@ static int role_fix_callback(hashtab_key_t key, hashtab_datum_t datum,
> > }
> >
> > ebitmap_init(&e_tmp);
> > - ebitmap_for_each_positive_bit(&role->dominates, rnode, i) {
> > - assert(mod->map[SYM_ROLES][i]);
> > - if (ebitmap_set_bit
> > - (&e_tmp, mod->map[SYM_ROLES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&role->dominates, &e_tmp, mod->map[SYM_ROLES]))
> > + goto cleanup;
> > if (ebitmap_union(&dest_role->dominates, &e_tmp)) {
> > goto cleanup;
> > }
> > - if (type_set_or_convert(&role->types, &dest_role->types, mod, state)) {
> > + if (type_set_or_convert(&role->types, &dest_role->types, mod)) {
> > goto cleanup;
> > }
> > ebitmap_destroy(&e_tmp);
> >
> > if (role->flavor == ROLE_ATTRIB) {
> > ebitmap_init(&e_tmp);
> > - ebitmap_for_each_positive_bit(&role->roles, rnode, i) {
> > - assert(mod->map[SYM_ROLES][i]);
> > - if (ebitmap_set_bit
> > - (&e_tmp, mod->map[SYM_ROLES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&role->roles, &e_tmp, mod->map[SYM_ROLES]))
> > + goto cleanup;
> > if (ebitmap_union(&dest_role->roles, &e_tmp)) {
> > goto cleanup;
> > }
> > @@ -1152,13 +1134,11 @@ static int role_fix_callback(hashtab_key_t key, hashtab_datum_t datum,
> > static int type_fix_callback(hashtab_key_t key, hashtab_datum_t datum,
> > void *data)
> > {
> > - unsigned int i;
> > char *id = key;
> > type_datum_t *type, *new_type = NULL;
> > link_state_t *state = (link_state_t *) data;
> > ebitmap_t e_tmp;
> > policy_module_t *mod = state->cur;
> > - ebitmap_node_t *tnode;
> > symtab_t *typetab;
> >
> > type = (type_datum_t *) datum;
> > @@ -1181,13 +1161,8 @@ static int type_fix_callback(hashtab_key_t key, hashtab_datum_t datum,
> > }
> >
> > ebitmap_init(&e_tmp);
> > - ebitmap_for_each_positive_bit(&type->types, tnode, i) {
> > - assert(mod->map[SYM_TYPES][i]);
> > - if (ebitmap_set_bit
> > - (&e_tmp, mod->map[SYM_TYPES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&type->types, &e_tmp, mod->map[SYM_TYPES]))
> > + goto cleanup;
> > if (ebitmap_union(&new_type->types, &e_tmp)) {
> > goto cleanup;
> > }
> > @@ -1269,9 +1244,8 @@ static int copy_avrule_list(avrule_t * list, avrule_t ** dst,
> > new_rule->specified = cur->specified;
> > new_rule->flags = cur->flags;
> > if (type_set_convert
> > - (&cur->stypes, &new_rule->stypes, module, state) == -1
> > - || type_set_convert(&cur->ttypes, &new_rule->ttypes, module,
> > - state) == -1) {
> > + (&cur->stypes, &new_rule->stypes, module) == -1
> > + || type_set_convert(&cur->ttypes, &new_rule->ttypes, module) == -1) {
> > goto cleanup;
> > }
> >
> > @@ -1355,8 +1329,6 @@ static int copy_role_trans_list(role_trans_rule_t * list,
> > policy_module_t * module, link_state_t * state)
> > {
> > role_trans_rule_t *cur, *new_rule = NULL, *tail;
> > - unsigned int i;
> > - ebitmap_node_t *cnode;
> >
> > cur = list;
> > tail = *dst;
> > @@ -1374,19 +1346,12 @@ static int copy_role_trans_list(role_trans_rule_t * list,
> > if (role_set_or_convert
> > (&cur->roles, &new_rule->roles, module, state)
> > || type_set_or_convert(&cur->types, &new_rule->types,
> > - module, state)) {
> > + module)) {
> > goto cleanup;
> > }
> >
> > - ebitmap_for_each_positive_bit(&cur->classes, cnode, i) {
> > - assert(module->map[SYM_CLASSES][i]);
> > - if (ebitmap_set_bit(&new_rule->classes,
> > - module->
> > - map[SYM_CLASSES][i] - 1,
> > - 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&cur->classes, &new_rule->classes, module->map[SYM_CLASSES]))
> > + goto cleanup;
> >
> > new_rule->new_role = module->map[SYM_ROLES][cur->new_role - 1];
> >
> > @@ -1476,8 +1441,8 @@ static int copy_filename_trans_list(filename_trans_rule_t * list,
> > if (!new_rule->name)
> > goto err;
> >
> > - if (type_set_or_convert(&cur->stypes, &new_rule->stypes, module, state) ||
> > - type_set_or_convert(&cur->ttypes, &new_rule->ttypes, module, state))
> > + if (type_set_or_convert(&cur->stypes, &new_rule->stypes, module) ||
> > + type_set_or_convert(&cur->ttypes, &new_rule->ttypes, module))
> > goto err;
> >
> > new_rule->tclass = module->map[SYM_CLASSES][cur->tclass - 1];
> > @@ -1497,8 +1462,6 @@ static int copy_range_trans_list(range_trans_rule_t * rules,
> > policy_module_t * mod, link_state_t * state)
> > {
> > range_trans_rule_t *rule, *new_rule = NULL;
> > - unsigned int i;
> > - ebitmap_node_t *cnode;
> >
> > for (rule = rules; rule; rule = rule->next) {
> > new_rule =
> > @@ -1512,21 +1475,15 @@ static int copy_range_trans_list(range_trans_rule_t * rules,
> > *dst = new_rule;
> >
> > if (type_set_convert(&rule->stypes, &new_rule->stypes,
> > - mod, state))
> > + mod))
> > goto cleanup;
> >
> > if (type_set_convert(&rule->ttypes, &new_rule->ttypes,
> > - mod, state))
> > + mod))
> > goto cleanup;
> >
> > - ebitmap_for_each_positive_bit(&rule->tclasses, cnode, i) {
> > - assert(mod->map[SYM_CLASSES][i]);
> > - if (ebitmap_set_bit
> > - (&new_rule->tclasses,
> > - mod->map[SYM_CLASSES][i] - 1, 1)) {
> > - goto cleanup;
> > - }
> > - }
> > + if (ebitmap_convert(&rule->tclasses, &new_rule->tclasses, mod->map[SYM_CLASSES]))
> > + goto cleanup;
> >
> > if (mls_range_convert(&rule->trange, &new_rule->trange, mod, state))
> > goto cleanup;
> > @@ -1688,15 +1645,12 @@ static int copy_scope_index(scope_index_t * src, scope_index_t * dest,
> > }
> > dest->class_perms_len = largest_mapped_class_value;
> > for (i = 0; i < src->class_perms_len; i++) {
> > - ebitmap_t *srcmap = src->class_perms_map + i;
> > + const ebitmap_t *srcmap = src->class_perms_map + i;
> > ebitmap_t *destmap =
> > dest->class_perms_map + module->map[SYM_CLASSES][i] - 1;
> > - ebitmap_for_each_positive_bit(srcmap, node, j) {
> > - if (ebitmap_set_bit(destmap, module->perm_map[i][j] - 1,
> > - 1)) {
> > - goto cleanup;
> > - }
> > - }
> > +
> > + if (ebitmap_convert(srcmap, destmap, module->perm_map[i]))
> > + goto cleanup;
> > }
> >
> > return 0;
> > --
> > 2.36.1
> >
