Re: [PATCH] coroutine-ucontext: Save fake stack for pooled coroutine

[Stefan Hajnoczi]

On Fri, Jan 12, 2024 at 07:36:19PM +0900, Akihiko Odaki wrote:
> Coroutine may be pooled even after COROUTINE_TERMINATE if
> CONFIG_COROUTINE_POOL is enabled and fake stack should be saved in
> such a case to keep AddressSanitizerUseAfterReturn working. Even worse,
> I'm seeing stack corruption without fake stack being saved.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  util/coroutine-ucontext.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)

Adding Marc-André Lureau and Lingfeng Yang, who authored the code in
question.

Stefan

> 
> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
> index 7b304c79d942..e62ced5d6779 100644
> --- a/util/coroutine-ucontext.c
> +++ b/util/coroutine-ucontext.c
> @@ -124,8 +124,9 @@ void start_switch_fiber_asan(CoroutineAction action, void 
> **fake_stack_save,
>  {
>  #ifdef CONFIG_ASAN
>      __sanitizer_start_switch_fiber(
> -            action == COROUTINE_TERMINATE ? NULL : fake_stack_save,
> -            bottom, size);
> +        !IS_ENABLED(CONFIG_COROUTINE_POOL) && action == COROUTINE_TERMINATE ?
> +            NULL : fake_stack_save,
> +        bottom, size);
>  #endif
>  }
>  
> @@ -269,10 +270,26 @@ static inline void 
> valgrind_stack_deregister(CoroutineUContext *co)
>  #endif
>  #endif
>  
> +#if defined(CONFIG_ASAN) && defined(CONFIG_COROUTINE_POOL)
> +static void coroutine_fn terminate(void *opaque)
> +{
> +    CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, opaque);
> +
> +    __sanitizer_start_switch_fiber(NULL, to->stack, to->stack_size);
> +    siglongjmp(to->env, COROUTINE_ENTER);
> +}
> +#endif
> +
>  void qemu_coroutine_delete(Coroutine *co_)
>  {
>      CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
>  
> +#if defined(CONFIG_ASAN) && defined(CONFIG_COROUTINE_POOL)
> +    co_->entry_arg = qemu_coroutine_self();
> +    co_->entry = terminate;
> +    qemu_coroutine_switch(co_->entry_arg, co_, COROUTINE_ENTER);
> +#endif
> +
>  #ifdef CONFIG_VALGRIND_H
>      valgrind_stack_deregister(co);
>  #endif
> 
> ---
> base-commit: f614acb7450282a119d85d759f27eae190476058
> change-id: 20240112-asan-eb695c769f40
> 
> Best regards,
> -- 
> Akihiko Odaki <akihiko.odaki@daynix.com>
>

signature.asc
Description: PGP signature