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

[Victor Toso]

Hi,

On Thu, Aug 04, 2016 at 01:42:06PM +0200, Pavel Grunt wrote:
> On Thu, 2016-08-04 at 13:38 +0200, Victor Toso wrote:
> > 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.
> 
> yeah, but the variable name is 'data' not 'buffer'

http://i.imgur.com/iWKad22.jpg

> > 
> > [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