From: James Carter <jwcart2@gmail.com>
To: "Christian Göttsche" <cgzones@googlemail.com>
Cc: selinux@vger.kernel.org
Subject: Re: [PATCH v3 1/8] libsepol: refactor ebitmap conversion in link.c
Date: Mon, 8 Aug 2022 11:01:59 -0400 [thread overview]
Message-ID: <CAP+JOzTEQa79dmOiN0CqStPEieo6QMjdaqYOrR9mPMH-r5yASQ@mail.gmail.com> (raw)
In-Reply-To: <20220721150515.19843-1-cgzones@googlemail.com>
On Thu, Jul 21, 2022 at 11:11 AM Christian Göttsche
<cgzones@googlemail.com> 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@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> 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
>
next prev parent reply other threads:[~2022-08-08 15:02 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-21 15:05 [PATCH v3 1/8] libsepol: refactor ebitmap conversion in link.c Christian Göttsche
2022-07-21 15:05 ` [PATCH v3 2/8] libsepol: add ebitmap iterator wrapper with startnode Christian Göttsche
2022-08-08 15:04 ` James Carter
2022-07-21 15:05 ` [PATCH v3 3/8] libsepol: add compile-time constraint for mutual exclusive attributes Christian Göttsche
2022-08-08 17:02 ` James Carter
2022-07-21 15:05 ` [PATCH v3 4/8] checkpolicy: add front-end support for segregate attributes Christian Göttsche
2022-08-08 17:09 ` James Carter
2023-08-11 16:38 ` Christian Göttsche
2023-08-11 19:48 ` James Carter
2022-07-21 15:05 ` [PATCH v3 5/8] libsepol/tests: add test " Christian Göttsche
2022-07-21 15:05 ` [PATCH v3 6/8] libsepol/cil: add support " Christian Göttsche
2022-08-08 17:15 ` James Carter
2022-07-21 15:05 ` [PATCH v3 7/8] secilc: run tests against development version of libsepol Christian Göttsche
2022-08-08 15:20 ` James Carter
2022-07-21 15:05 ` [PATCH v3 8/8] secilc: include segregate attributes in tests Christian Göttsche
2022-08-08 15:01 ` James Carter [this message]
2022-08-09 15:22 ` [PATCH v3 1/8] libsepol: refactor ebitmap conversion in link.c 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=CAP+JOzTEQa79dmOiN0CqStPEieo6QMjdaqYOrR9mPMH-r5yASQ@mail.gmail.com \
--to=jwcart2@gmail.com \
--cc=cgzones@googlemail.com \
--cc=selinux@vger.kernel.org \
/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.