[PATCH 0/3] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()

Stefan Hajnoczi posted 3 patches 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220307153853.602859-1-stefanha@redhat.com
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Stefan Weil <sw@weilnetz.de>
util/coroutine-ucontext.c | 38 +++++++++++++++++++++++-------------
util/coroutine-win32.c    | 18 ++++++++++++-----
util/qemu-coroutine.c     | 41 +++++++++++++++++++++++----------------
3 files changed, 61 insertions(+), 36 deletions(-)
[PATCH 0/3] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
Posted by Stefan Hajnoczi 2 years, 2 months ago
The coroutine implementation uses __thread variables internally. Compiler
optimizations may cache Thread-Local Storage values across
qemu_coroutine_yield(), leading to stale values being used after the coroutine
is re-entered from another thread.

Kevin pointed out that the coroutine implementation itself is vulnerable to
this problem. As a follow-up to my coroutine TLS patch series I'm sending these
patches to convert __thread variables to the new "qemu/coroutine-tls.h" macros
so they are safe.

Stefan Hajnoczi (3):
  coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS()
  coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
  coroutine-win32: use QEMU_DEFINE_STATIC_CO_TLS()

 util/coroutine-ucontext.c | 38 +++++++++++++++++++++++-------------
 util/coroutine-win32.c    | 18 ++++++++++++-----
 util/qemu-coroutine.c     | 41 +++++++++++++++++++++++----------------
 3 files changed, 61 insertions(+), 36 deletions(-)

-- 
2.35.1

Re: [PATCH 0/3] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
Posted by Kevin Wolf 2 years ago
Am 07.03.2022 um 16:38 hat Stefan Hajnoczi geschrieben:
> The coroutine implementation uses __thread variables internally. Compiler
> optimizations may cache Thread-Local Storage values across
> qemu_coroutine_yield(), leading to stale values being used after the coroutine
> is re-entered from another thread.
> 
> Kevin pointed out that the coroutine implementation itself is vulnerable to
> this problem. As a follow-up to my coroutine TLS patch series I'm sending these
> patches to convert __thread variables to the new "qemu/coroutine-tls.h" macros
> so they are safe.

Thanks, applied to the block branch.

Kevin
Re: [PATCH 0/3] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
Posted by Philippe Mathieu-Daudé 2 years, 2 months ago
On 7/3/22 16:38, Stefan Hajnoczi wrote:
> The coroutine implementation uses __thread variables internally. Compiler
> optimizations may cache Thread-Local Storage values across
> qemu_coroutine_yield(), leading to stale values being used after the coroutine
> is re-entered from another thread.
> 
> Kevin pointed out that the coroutine implementation itself is vulnerable to
> this problem. As a follow-up to my coroutine TLS patch series I'm sending these
> patches to convert __thread variables to the new "qemu/coroutine-tls.h" macros
> so they are safe.
> 
> Stefan Hajnoczi (3):
>    coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS()
>    coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
>    coroutine-win32: use QEMU_DEFINE_STATIC_CO_TLS()

Series:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 0/3] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
Posted by Stefan Hajnoczi 2 years, 1 month ago
On Mon, Mar 07, 2022 at 03:38:50PM +0000, Stefan Hajnoczi wrote:
> The coroutine implementation uses __thread variables internally. Compiler
> optimizations may cache Thread-Local Storage values across
> qemu_coroutine_yield(), leading to stale values being used after the coroutine
> is re-entered from another thread.
> 
> Kevin pointed out that the coroutine implementation itself is vulnerable to
> this problem. As a follow-up to my coroutine TLS patch series I'm sending these
> patches to convert __thread variables to the new "qemu/coroutine-tls.h" macros
> so they are safe.
> 
> Stefan Hajnoczi (3):
>   coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS()
>   coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
>   coroutine-win32: use QEMU_DEFINE_STATIC_CO_TLS()
> 
>  util/coroutine-ucontext.c | 38 +++++++++++++++++++++++-------------
>  util/coroutine-win32.c    | 18 ++++++++++++-----
>  util/qemu-coroutine.c     | 41 +++++++++++++++++++++++----------------
>  3 files changed, 61 insertions(+), 36 deletions(-)

Kevin: Is this what you had in mind?