Hi,

On 07/16/2014 01:24 AM, Fabiano FidĂȘncio wrote:
> ---
>  usbredirhost/usbredirhost.c             | 1 +
>  usbredirtestclient/usbredirtestclient.c | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> index bbaafa4..6ab6e1b 100644
> --- a/usbredirhost/usbredirhost.c
> +++ b/usbredirhost/usbredirhost.c
> @@ -1191,6 +1191,7 @@ static void usbredirhost_alloc_stream_unlocked(struct 
> usbredirhost *host,
>                  host->endpoint[EP2I(ep)].transfer[i], INTERRUPT_TIMEOUT);
>              break;
>          }
> +        free(buffer);
>      }
>      host->endpoint[EP2I(ep)].out_idx = 0;
>      host->endpoint[EP2I(ep)].drop_packets = 0;

Erm, no this wrong, very very wrong, you're freeing the buffer were using to 
store the actual
usb data in here, while the usb transfers are still being used (are being 
created to get
used) not good.

This will get freed when the actual transfer gets free-ed through calling
usbredirhost_free_transfer(), specifically by this line in that function:

    free(transfer->transfer->buffer);

> diff --git a/usbredirtestclient/usbredirtestclient.c 
> b/usbredirtestclient/usbredirtestclient.c
> index 42b16dc..32fcba2 100644
> --- a/usbredirtestclient/usbredirtestclient.c
> +++ b/usbredirtestclient/usbredirtestclient.c
> @@ -404,6 +404,9 @@ static int usbredirtestclient_cmdline_ctrl(void)
>      }
>      usbredirparser_send_control_packet(parser, id, &control_packet,
>                                         data, data_len);
> +    if (data) {
> +        free(data);
> +    }
>      printf("Send control packet with id: %u\n", id);
>      id++;
>      return 1;

This leak fix is correct, no need for the if though, free(NULL) is a nop. I've 
pushed this
to the usbredir git repo.

Regards,

Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to