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