On Fri, Jan 28, 2022 at 3:29 PM <vbendel@xxxxxxxxxx> wrote:
> From: Vratislav Bendel <vbendel@xxxxxxxxxx>
>
> On error path from cond_read_list() and duplicate_policydb_cond_list()
> the *_destroy() functions get called a second time in caller functions.
> Remove the first calls and let the callers clean it.
>
> Suggested-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> Signed-off-by: Vratislav Bendel <vbendel@xxxxxxxxxx>
> ---
> security/selinux/ss/conditional.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 8bc16ad3af9e..c333daaeceab 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -432,19 +432,16 @@ int cond_read_list(struct policydb *p, void *fp)
>
> rc = avtab_alloc(&(p->te_cond_avtab), p->te_avtab.nel);
> if (rc)
> - goto err;
> + return rc;
>
> p->cond_list_len = len;
>
> for (i = 0; i < len; i++) {
> rc = cond_read_node(p, &p->cond_list[i], fp);
> if (rc)
> - goto err;
> + return rc;
> }
> return 0;
> -err:
> - cond_list_destroy(p);
> - return rc;
> }
I tend to prefer functions that cleanup their own allocations on
error. It makes it easier and quicker to reason about a function's
error handling. I recognize in this case it may mean multiple calls
to cond_list_destroy(), but that should be safe (considering the
previous patches in this series), and we are on the error path anyway
so I'm not as worried about a few extra instructions.
--
paul-moore.com
