Re: [XEN PATCH v9 02/24] xen/arm: add TEE teardown to arch_domain_teardown()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/07/2023 10:34 am, Jens Wiklander wrote:
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 15d9709a97d2..18171decdc66 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -795,6 +795,42 @@ fail:
>  
>  int arch_domain_teardown(struct domain *d)
>  {
> +    int ret = 0;
> +
> +    BUG_ON(!d->is_dying);
> +
> +    /* See domain_teardown() for an explanation of all of this magic. */
> +    switch ( d->teardown.arch_val )
> +    {
> +#define PROGRESS(x)                             \
> +        d->teardown.arch_val = PROG_ ## x;      \
> +        fallthrough;                            \
> +    case PROG_ ## x
> +
> +        enum {
> +            PROG_none,
> +            PROG_tee,
> +            PROG_done,
> +        };
> +
> +    case PROG_none:
> +        BUILD_BUG_ON(PROG_none != 0);
> +
> +    PROGRESS(tee):
> +        ret = tee_domain_teardown(d);
> +        if ( ret )
> +            return ret;
> +        break;

This unconditional break isn't correct.

The logic functions right now (because upon hitting return 0, you don't
re-enter this function), but will cease working when you add a new
PROG_*, or when the calling code gets more complicated.

> +
> +    PROGRESS(done):
> +        break;

This needs to be the only break in the switch statement, for it to
behave in the intended manner.

~Andrew