Am 05.07.2014 um 22:06 hat Max Reitz geschrieben:
> 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 <kw...@redhat.com>
> >---
> > 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.
Okay, I'll add the suggested comment and assertion.
> Speaking of leaks: Shouldn't the backing_qiov be freed in
> qed_aio_complete()?
I can't see a code path where cb (i.e. qed_aio_next_io or
qed_copy_from_backing_file_write) wouldn't be called before
reaching qed_aio_complete(). Both of them free backing_qiov.
Kevin
