Re: [PATCH] block/io: fix bdrv_co_do_copy_on_readv

[Vladimir Sementsov-Ogievskiy]

13.03.2020 2:09, John Snow wrote:
> On 3/12/20 4:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Prior to 1143ec5ebf4 it was OK to qemu_iovec_from_buf() from aligned-up
> > buffer to original qiov, as qemu_iovec_from_buf() will stop at qiov end
> > anyway.
> >
> > But after 1143ec5ebf4 we assume that bdrv_co_do_copy_on_readv works on
> > part of original qiov, defined by qiov_offset and bytes. So we must not
> > touch qiov behind qiov_offset+bytes bound. Fix it.
>
> For the purposes of the stable branch commit log, how does the bug
> manifest? Are there known cases? What's the impact?
>
> (Do we have tests?)

Sorry, nothing of these things. I just saw it while working with this code.

> > Cc: address@hidden # v4.2
> > Fixes: 1143ec5ebf4
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> >   block/io.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/io.c b/block/io.c
> > index 7e4cb74cf4..aba67f66b9 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -1399,7 +1399,7 @@ static int coroutine_fn 
> > bdrv_co_do_copy_on_readv(BdrvChild *child,
> >               if (!(flags & BDRV_REQ_PREFETCH)) {
> >                   qemu_iovec_from_buf(qiov, qiov_offset + progress,
> >                                       bounce_buffer + skip_bytes,
> > -                                    pnum - skip_bytes);
> > +                                    MIN(pnum - skip_bytes, bytes - progress));
> >               }
> >           } else if (!(flags & BDRV_REQ_PREFETCH)) {
> >               /* Read directly into the destination */
> Even if I don't understand the bug, the tighter bound seems provably
> correct anyway, so...
>
> Reviewed-by: John Snow <address@hidden>

Thanks!

--
Best regards,
Vladimir