From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] [PATCH] segfault due to buffer overrun in usb-serial
Date: Wed, 10 Feb 2010 13:28:24 -0600
Message-ID: <4B7308D8.3000801@codemonkey.ws>
References: <4B699DB6.4090604@cisco.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: qemu-devel@nongnu.org, kvm-devel <kvm@vger.kernel.org>
To: "David S. Ahern" <daahern@cisco.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-iw0-f185.google.com ([209.85.223.185]:65249 "EHLO
	mail-iw0-f185.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754180Ab0BJT2a (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 10 Feb 2010 14:28:30 -0500
Received: by iwn15 with SMTP id 15so403726iwn.19
        for <kvm@vger.kernel.org>; Wed, 10 Feb 2010 11:28:30 -0800 (PST)
In-Reply-To: <4B699DB6.4090604@cisco.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 02/03/2010 10:00 AM, David S. Ahern wrote:
> This fixes a segfault due to buffer overrun in the usb-serial device.
> The memcpy was incrementing the start location by recv_used yet, the
> computation of first_size (how much to write at the end of the buffer
> before wrapping to the front) was not accounting for it. This causes the
> next element after the receive buffer (recv_ptr) to get overwritten with
> random data.
>
> Signed-off-by: David Ahern<daahern@cisco.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> diff --git a/hw/usb-serial.c b/hw/usb-serial.c
> index 37293ea..c3f3401 100644
> --- a/hw/usb-serial.c
> +++ b/hw/usb-serial.c
> @@ -497,12 +497,28 @@ static int usb_serial_can_read(void *opaque)
>   static void usb_serial_read(void *opaque, const uint8_t *buf, int size)
>   {
>       USBSerialState *s = opaque;
> -    int first_size = RECV_BUF - s->recv_ptr;
> -    if (first_size>  size)
> -        first_size = size;
> -    memcpy(s->recv_buf + s->recv_ptr + s->recv_used, buf, first_size);
> -    if (size>  first_size)
> -        memcpy(s->recv_buf, buf + first_size, size - first_size);
> +    int first_size, start;
> +
> +    /* room in the buffer? */
> +    if (size>  (RECV_BUF - s->recv_used))
> +        size = RECV_BUF - s->recv_used;
> +
> +    start = s->recv_ptr + s->recv_used;
> +    if (start<  RECV_BUF) {
> +        /* copy data to end of buffer */
> +        first_size = RECV_BUF - start;
> +        if (first_size>  size)
> +            first_size = size;
> +
> +        memcpy(s->recv_buf + start, buf, first_size);
> +
> +        /* wrap around to front if needed */
> +        if (size>  first_size)
> +            memcpy(s->recv_buf, buf + first_size, size - first_size);
> +    } else {
> +        start -= RECV_BUF;
> +        memcpy(s->recv_buf + start, buf, size);
> +    }
>       s->recv_used += size;
>   }
>
>
>
>
>
>