Re: [Qemu-devel] [PATCH] sheepdog: discard the payload if the header is invalid

[Jeff Cody <jcody@redhat.com>]

On Tue, Sep 01, 2015 at 09:29:31AM +0800, Liu Yuan wrote:
> From: Liu Yuan <liuyuan@cmss.chinamobile.com>
> 
> We need to discard the payload if we get a invalid header due to whatever reason
> to avoid data stream curruption.

If the header is invalid / corrupted, how can rsp.data_length be
trusted?  Out of curiosity, is this an issue you are seeing occur "in
the wild"?

> For e.g., the response consists of header plus
> data payload. If we simply read the header then the data payload is left in the
> socket buffer and the next time we would read the garbage data and currupt the
> whole connection.
> 
> Cc: qemu-devel@nongnu.org
> Cc: Jeff Cody <jcody@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Liu Yuan <liuyuan@cmss.chinamobile.com>
> ---
>  block/sheepdog.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 9585beb..9ed3458 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -794,6 +794,14 @@ static void coroutine_fn aio_read_response(void *opaque)
>          }
>      }
>      if (!aio_req) {
> +        if (rsp.data_length) {
> +            void *garbage = g_malloc(rsp.data_length);
> +            ret = qemu_co_recv(fd, garbage, rsp.data_length);
> +            if (ret != rsp.data_length) {
> +                error_report("failed to discard the data, %s", strerror(errno));
> +            }
> +            g_free(garbage);
> +        }
>          error_report("cannot find aio_req %x", rsp.id);
>          goto err;
>      }
> -- 
> 1.9.1
>