Re: [PATCH v2] 9p: Lock directory streams with a CoMutex

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Montag, 18. Mai 2020 18:35:04 CEST Greg Kurz wrote:
> On Mon, 18 May 2020 18:22:28 +0200
> 
> Greg Kurz <groug@kaod.org> wrote:
> > Locking was introduced in QEMU 2.7 to address the deprecation of
> > readdir_r(3) in glibc 2.24. It turns out that the frontend code is
> > the worst place to handle a critical section with a pthread mutex:
> > the code runs in a coroutine on behalf of the QEMU mainloop and then
> > yields control, waiting for the fsdev backend to process the request
> > in a worker thread. If the client resends another readdir request for
> > the same fid before the previous one finally unlocked the mutex, we're
> > deadlocked.
> > 
> > This never bit us because the linux client serializes readdir requests
> > for the same fid, but it is quite easy to demonstrate with a custom
> > client.
> > 
> > A good solution could be to narrow the critical section in the worker
> > thread code and to return a copy of the dirent to the frontend, but
> > this causes quite some changes in both 9p.c and codir.c. So, instead
> > of that, in order for people to easily backport the fix to older QEMU
> > versions, let's simply use a CoMutex since all the users for this
> > sit in coroutines.
> > 
> > Fixes: 7cde47d4a89d ("9p: add locking to V9fsDir")
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> 
> Just to clarify, this is v2 of:
> 
> 9pfs: Fix potential deadlock of QEMU mainloop
> 
> https://patchwork.ozlabs.org/project/qemu-devel/patch/158826201391.1344781.9
> 403916162733181811.stgit@bahia.lan/

Good move!

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>