[Spice-devel] [usbredir v1] usbredirhost: fix leak on error

[Victor Toso]

Hi,

On Thu, Aug 04, 2016 at 08:37:02AM +0200, Pavel Grunt wrote:
> Hey
> 
> On Wed, 2016-08-03 at 18:11 +0200, Victor Toso wrote:
> > Pointed by coverity:
> >  17. usbredir-0.7.1/usbredirhost/usbredirhost.c:2306: leaked_storage:
> >  Returning without freeing "data" leaks the storage that it points to.
> >  #  2304|
> >  #  usbredirhost_bulk_packet_complete(transfer->transfer);
> >  #  2305|       }
> >  #  2306|-> }
> >  #  2307|
> >  #  2308|   static void usbredirhost_iso_packet(void *priv, uint64_t
> >  #  id,
> > ---
> >  usbredirhost/usbredirhost.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> > index 3e80677..2b3ee74 100644
> > --- a/usbredirhost/usbredirhost.c
> > +++ b/usbredirhost/usbredirhost.c
> > @@ -2280,6 +2280,7 @@ static void usbredirhost_bulk_packet(void *priv,
> > uint64_t id,
> >                                           transfer, BULK_TIMEOUT);
> >  #else
> >          r = LIBUSB_ERROR_INVALID_PARAM;
> > +        free(buffer);
> buffer is not declared in that function

But the malloc is ours [0] and we free it in situation of error when
usbredirhost_alloc_transfer() fails.

[0] https://cgit.freedesktop.org/spice/usbredir/tree/usbredirhost/usbredirhost.c#n2256

Not sure about the code design but it seems we should free the data if
we don't pass this buffer to libusb_fill_bulk_transfer()

Cheers,
  toso

>
> Pavel
> >          goto error;
> >  #endif
> >      } else {
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel