Re: [PATCH] 9pfs: Fix potential deadlock of QEMU mainloop

[Christian Schoenebeck]

On Mittwoch, 6. Mai 2020 19:54:15 CEST Greg Kurz wrote:
> On Wed, 06 May 2020 15:36:16 +0200
> 
> Christian Schoenebeck <address@hidden> wrote:
> > On Mittwoch, 6. Mai 2020 15:05:23 CEST Christian Schoenebeck wrote:
> > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > > index 9e046f7acb51..ac84ae804496 100644
> > > > --- a/hw/9pfs/9p.c
> > > > +++ b/hw/9pfs/9p.c
> > > > @@ -2170,7 +2170,7 @@ static int coroutine_fn
> > > > v9fs_do_readdir_with_stat(V9fsPDU *pdu, int32_t count = 0;
> > > > 
> > > >      struct stat stbuf;
> > > >      off_t saved_dir_pos;
> > > > 
> > > > -    struct dirent *dent;
> > > > +    struct dirent dent;
> > 
> > One more: since this dirent structure is now on the stack, it should
> > better be initialized for safety reasons.
> 
> I don't think so, for two reasons:
> - I can't think of an initializer that would make sense for a dirent

The same as it would (implied - usually, e.g. with gmalloc0()) if you were 
allocating it on heap: by initializing it with all zeroes, e.g. just:

        struct dirent dent = {};

> - if a future change introduces a branch where dent could be used
>   uninitialized, I'd rather give a chance to the compiler to bark

The compiler would reliably bark on using it unitialized if you were about to 
access it directly within the same function. But that's not the case here. 
dirent is passed by reference to a function which will be altering it. Such 
stacked relations usually require more sophisticated diagnostics, like e.g. 
exeuting the LLVM sanitizer.

Best regards,
Christian Schoenebeck