On Mon, Jun 7, 2010 at 5:07 PM, Dan Carpenter <error27@xxxxxxxxx> wrote:
> Originally cond_read_node() returned -1 (-EPERM) on errors which was
> incorrect. Now it either propagates the error codes from lower level
> functions next_entry() or cond_read_av_list() or it returns -ENOMEM or
> -EINVAL.
>
> next_entry() returns -EINVAL.
> cond_read_av_list() returns -EINVAL or -ENOMEM.
>
> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
>
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 4c39f19..f0de637 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -392,14 +392,14 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
>
> rc = next_entry(buf, fp, sizeof(u32));
> if (rc < 0)
> - return -1;
> + return rc;
>
> node->cur_state = le32_to_cpu(buf[0]);
>
> len = 0;
> rc = next_entry(buf, fp, sizeof(u32));
> if (rc < 0)
> - return -1;
> + return rc;
>
> /* expr */
> len = le32_to_cpu(buf[0]);
> @@ -409,6 +409,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
> if (rc < 0)
> goto err;
>
> + rc = -ENOMEM;
> expr = kzalloc(sizeof(struct cond_expr), GFP_KERNEL);
> if (!expr)
> goto err;
> @@ -417,6 +418,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
> expr->bool = le32_to_cpu(buf[1]);
>
> if (!expr_isvalid(p, expr)) {
> + rc = -EINVAL;
> kfree(expr);
> goto err;
> }
> @@ -428,14 +430,16 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
> last = expr;
> }
>
> - if (cond_read_av_list(p, fp, &node->true_list, NULL) != 0)
> + rc = cond_read_av_list(p, fp, &node->true_list, NULL);
> + if (rc < 0)
> goto err;
> - if (cond_read_av_list(p, fp, &node->false_list, node->true_list) != 0)
> + rc = cond_read_av_list(p, fp, &node->false_list, node->true_list);
> + if (rc < 0)
> goto err;
I know that lots of the next_entry() calls use if (rc < 0) [unrelated
note I think those need to be fixed too] but most of the code uses if
(rc) and I strongly prefer if (rc) as it's cleaner and faster from a
micro architecture point of view. Would you mind re-spinning this
one?
-Eric
> return 0;
> err:
> cond_node_destroy(node);
> - return -1;
> + return rc;
> }
>
> int cond_read_list(struct policydb *p, void *fp)
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
