Re: [Qemu-devel] [PATCH 3/4] qed: Make qiov match request size until backing file EOF

[Max Reitz]

On 04.07.2014 17:55, Kevin Wolf wrote:
> If a QED image has a shorter backing file and a read request to
> unallocated clusters goes across EOF of the backing file, the backing
> file sees a shortened request and the rest is filled with zeros.
> However, the original too long qiov was used with the shortened request.
>
> This patch makes the qiov size match the request size, avoiding a
> potential buffer overflow in raw-posix.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>   block/qed.c | 26 +++++++++++++++++++++++---
>   block/qed.h |  1 +
>   2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/block/qed.c b/block/qed.c
> index b69374b..1f63b8f 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -772,6 +772,7 @@ static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
>    */
>   static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
>                                     QEMUIOVector *qiov,
> +                                  QEMUIOVector **backing_qiov,

This could be documented in the comment above the function header.

>                                     BlockDriverCompletionFunc *cb, void *opaque)
>   {
>       uint64_t backing_length = 0;
> @@ -804,15 +805,20 @@ static void qed_read_backing_file(BDRVQEDState *s, 
> uint64_t pos,
>       /* If the read straddles the end of the backing file, shorten it */
>       size = MIN((uint64_t)backing_length - pos, qiov->size);
>   
> +    *backing_qiov = g_new(QEMUIOVector, 1);

I guess at least because of the qemu_iovec_destroy() block in 
qed_aio_next_io() *backing_qiov always has to be NULL before this point. 
I guess I'd like an assert(!*backing_qiov) here (or 
assert(!acb->backing_qiov) before the call to qed_read_backing_file() in 
qed_aio_read_data()) anyway to express clearly that there can be no leaks.

Speaking of leaks: Shouldn't the backing_qiov be freed in 
qed_aio_complete()?

Max