* [PATCH] 9pfs: fix crash in v9fs_walk()
@ 2021-09-01 16:15 Christian Schoenebeck
2021-09-01 16:47 ` [SPAM] " Greg Kurz
0 siblings, 1 reply; 3+ messages in thread
From: Christian Schoenebeck @ 2021-09-01 16:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, qemu-stable
v9fs_walk() utilizes the v9fs_co_run_in_worker({...}) macro to run the
supplied fs driver code block on a background worker thread.
When either the 'Twalk' client request was interrupted or if the client
requested fid for that 'Twalk' request caused a stat error then that
fs driver code block was left by 'break' keyword, with the intention to
return from worker thread back to main thread as well:
v9fs_co_run_in_worker({
if (v9fs_request_cancelled(pdu)) {
err = -EINTR;
break;
}
err = s->ops->lstat(&s->ctx, &dpath, &fidst);
if (err < 0) {
err = -errno;
break;
}
...
});
However that 'break;' statement also skipped the v9fs_co_run_in_worker()
macro's final and mandatory
/* re-enter back to qemu thread */
qemu_coroutine_yield();
call and thus caused the rest of v9fs_walk() to be continued being
executed on the worker thread instead of main thread, eventually
leading to a crash in the transport virtio transport driver.
To fix this issue and to prevent the same error from happening again by
other users of v9fs_co_run_in_worker() in future, auto wrap the supplied
code block into its own
do { } while (0);
loop inside the 'v9fs_co_run_in_worker' macro definition.
Full discussion and backtrace:
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg05209.html
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg00174.html
Fixes: 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: qemu-stable@nongnu.org
---
hw/9pfs/coth.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index c51289903d..f83c7dda7b 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -51,7 +51,9 @@
*/ \
qemu_coroutine_yield(); \
qemu_bh_delete(co_bh); \
- code_block; \
+ do { \
+ code_block; \
+ } while (0); \
/* re-enter back to qemu thread */ \
qemu_coroutine_yield(); \
} while (0)
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [SPAM] [PATCH] 9pfs: fix crash in v9fs_walk()
2021-09-01 16:15 [PATCH] 9pfs: fix crash in v9fs_walk() Christian Schoenebeck
@ 2021-09-01 16:47 ` Greg Kurz
2021-09-01 17:22 ` Christian Schoenebeck
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kurz @ 2021-09-01 16:47 UTC (permalink / raw)
To: Christian Schoenebeck; +Cc: qemu-devel, qemu-stable
On Wed, 1 Sep 2021 18:15:10 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> v9fs_walk() utilizes the v9fs_co_run_in_worker({...}) macro to run the
> supplied fs driver code block on a background worker thread.
>
> When either the 'Twalk' client request was interrupted or if the client
> requested fid for that 'Twalk' request caused a stat error then that
> fs driver code block was left by 'break' keyword, with the intention to
> return from worker thread back to main thread as well:
>
> v9fs_co_run_in_worker({
> if (v9fs_request_cancelled(pdu)) {
> err = -EINTR;
> break;
> }
> err = s->ops->lstat(&s->ctx, &dpath, &fidst);
> if (err < 0) {
> err = -errno;
> break;
> }
> ...
> });
>
> However that 'break;' statement also skipped the v9fs_co_run_in_worker()
> macro's final and mandatory
>
> /* re-enter back to qemu thread */
> qemu_coroutine_yield();
>
> call and thus caused the rest of v9fs_walk() to be continued being
> executed on the worker thread instead of main thread, eventually
> leading to a crash in the transport virtio transport driver.
>
> To fix this issue and to prevent the same error from happening again by
> other users of v9fs_co_run_in_worker() in future, auto wrap the supplied
> code block into its own
>
> do { } while (0);
>
> loop inside the 'v9fs_co_run_in_worker' macro definition.
>
> Full discussion and backtrace:
> https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg05209.html
> https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg00174.html
>
> Fixes: 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Cc: qemu-stable@nongnu.org
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/9pfs/coth.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
> index c51289903d..f83c7dda7b 100644
> --- a/hw/9pfs/coth.h
> +++ b/hw/9pfs/coth.h
> @@ -51,7 +51,9 @@
> */ \
> qemu_coroutine_yield(); \
> qemu_bh_delete(co_bh); \
> - code_block; \
> + do { \
> + code_block; \
> + } while (0); \
> /* re-enter back to qemu thread */ \
> qemu_coroutine_yield(); \
> } while (0)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] 9pfs: fix crash in v9fs_walk()
2021-09-01 16:47 ` [SPAM] " Greg Kurz
@ 2021-09-01 17:22 ` Christian Schoenebeck
0 siblings, 0 replies; 3+ messages in thread
From: Christian Schoenebeck @ 2021-09-01 17:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, qemu-stable
On Mittwoch, 1. September 2021 18:47:21 CEST Greg Kurz wrote:
> On Wed, 1 Sep 2021 18:15:10 +0200
>
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > v9fs_walk() utilizes the v9fs_co_run_in_worker({...}) macro to run the
> > supplied fs driver code block on a background worker thread.
> >
> > When either the 'Twalk' client request was interrupted or if the client
> > requested fid for that 'Twalk' request caused a stat error then that
> > fs driver code block was left by 'break' keyword, with the intention to
> >
> > return from worker thread back to main thread as well:
> > v9fs_co_run_in_worker({
> >
> > if (v9fs_request_cancelled(pdu)) {
> >
> > err = -EINTR;
> > break;
> >
> > }
> > err = s->ops->lstat(&s->ctx, &dpath, &fidst);
> > if (err < 0) {
> >
> > err = -errno;
> > break;
> >
> > }
> > ...
> >
> > });
> >
> > However that 'break;' statement also skipped the v9fs_co_run_in_worker()
> > macro's final and mandatory
> >
> > /* re-enter back to qemu thread */
> > qemu_coroutine_yield();
> >
> > call and thus caused the rest of v9fs_walk() to be continued being
> > executed on the worker thread instead of main thread, eventually
> > leading to a crash in the transport virtio transport driver.
> >
> > To fix this issue and to prevent the same error from happening again by
> > other users of v9fs_co_run_in_worker() in future, auto wrap the supplied
> > code block into its own
> >
> > do { } while (0);
> >
> > loop inside the 'v9fs_co_run_in_worker' macro definition.
> >
> > Full discussion and backtrace:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg05209.html
> > https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg00174.html
> >
> > Fixes: 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Cc: qemu-stable@nongnu.org
> > ---
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next
Thanks!
I'll send out a PR tomorrow.
> > hw/9pfs/coth.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
> > index c51289903d..f83c7dda7b 100644
> > --- a/hw/9pfs/coth.h
> > +++ b/hw/9pfs/coth.h
> > @@ -51,7 +51,9 @@
> >
> > */ \
> >
> > qemu_coroutine_yield(); \
> > qemu_bh_delete(co_bh); \
> >
> > - code_block; \
> > + do { \
> > + code_block; \
> > + } while (0); \
> >
> > /* re-enter back to qemu thread */ \
> > qemu_coroutine_yield(); \
> >
> > } while (0)
Best regards,
Christian Schoenebeck
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-01 17:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 16:15 [PATCH] 9pfs: fix crash in v9fs_walk() Christian Schoenebeck
2021-09-01 16:47 ` [SPAM] " Greg Kurz
2021-09-01 17:22 ` Christian Schoenebeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).