From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Missing unlock in error path.

Fixes: Covertiy CID 1413123
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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;
         }
-- 
2.24.1

Hi David,

On 2/4/20 12:05 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Missing unlock in error path.
> 
> Fixes: Covertiy CID 1413123
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   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) {
              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,
---

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Hi David,
> 
> On 2/4/20 12:05 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Missing unlock in error path.
> > 
> > Fixes: Covertiy CID 1413123
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >   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

>              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 / dgilbert@redhat.com / Manchester, UK

On 2/4/20 4:44 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> Hi David,
>>
>> On 2/4/20 12:05 PM, Dr. David Alan Gilbert (git) wrote:
>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>>
>>> Missing unlock in error path.
>>>
>>> Fixes: Covertiy CID 1413123
>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> ---
>>>    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é <philmd@redhat.com>

> 
>>               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 / dgilbert@redhat.com / Manchester, UK
> 
>