Re: [PATCH 3/4] virtiofsd: load_capng missing unlock

[Philippe Mathieu-Daudé]

On 2/4/20 4:44 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (address@hidden) wrote:
> > Hi David,
> >
> > On 2/4/20 12:05 PM, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <address@hidden>
> > >
> > > Missing unlock in error path.
> > >
> > > Fixes: Covertiy CID 1413123
> > > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > > ---
> > >    tools/virtiofsd/passthrough_ll.c | 1 +
> > >    1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > > index e6f2399efc..c635fc8820 100644
> > > --- a/tools/virtiofsd/passthrough_ll.c
> > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > @@ -232,6 +232,7 @@ static int load_capng(void)
> > >             */
> > >            cap.saved = capng_save_state();
> > >            if (!cap.saved) {
> > > +            pthread_mutex_unlock(&cap.mutex);
> > >                fuse_log(FUSE_LOG_ERR, "capng_save_state (thread)\n");
> > >                return -EINVAL;
> > >            }
> >
> > What about moving the unlock call?
> >
> > -- >8 --
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -231,11 +231,11 @@ static int load_capng(void)
> >            * so make another.
> >            */
> >           cap.saved = capng_save_state();
> > +        pthread_mutex_unlock(&cap.mutex);
> >           if (!cap.saved) {
>
> I don't think I can legally check cap.saved there if I've already
> unlocked

Sorry I was with low sugar... I read it as a copy.

The patch is fine:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> >               fuse_log(FUSE_LOG_ERR, "capng_save_state (thread)\n");
> >               return -EINVAL;
> >           }
> > -        pthread_mutex_unlock(&cap.mutex);
> >
> >           /*
> >            * We want to use the loaded state for our pid,
> > ---
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK