Re: [PATCH] 9pfs: fix crash in v9fs_walk()

[Christian Schoenebeck]

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