Hi, Daniel.
Thank you for your comment.
> > @@ -119,8 +126,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> > int amtread = -1;
> > int ret = 0;
> > unsigned long long remain;
> > - size_t rbytes = 1024 * 1024; /* For Read */
> > - char zerobuf[512];
> > + size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
> > + size_t wbytes = WRITE_BLOCK_SIZE_DEFAULT;
> > + int interval;
> > + char zerobuf[WRITE_BLOCK_SIZE_DEFAULT];
>
> Here there is a fixed size buffer which will be written...
>
> > char *buf = NULL;
> >
> > if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) {
> > @@ -131,6 +140,12 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> > goto cleanup;
> > }
> >
> > +#ifdef __linux__
> > + if (ioctl(fd, BLKBSZGET, &wbytes) < 0) {
> > + wbytes = WRITE_BLOCK_SIZE_DEFAULT;
> > + }
> > +#endif
>
> ...but the amount written is variable. So this will have an
> out of bounds array access if the returned 'wbytes' is greater
> than WRITE_BLOCK_SIZE_DEFAULT. I think you need to change
> 'zerobuf' to be a heap-allocated variable, instead of fixed
> sized stack allocated variable. eg
>
> char *zerobuf;
> if (VIR_ALLOC_N(zerobuf, wbytes) < 0) {
> virReportOOMError();
> goto cleanup;
> }
> ....
Ouch, I make a big mistake.
You are right. I'll remake a patch.
> > +
> > bzero(&zerobuf, sizeof(zerobuf));
>
> VIR_ALLOC_N zeros, so this line could then be removed.
OK. I'll remove this.
> >
> > if (VIR_ALLOC_N(buf, rbytes) < 0) {
> > @@ -160,7 +175,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> > * blocks */
> > amtleft = amtread;
> > do {
> > - int interval = ((512 > amtleft) ? amtleft : 512);
> > + interval = ((wbytes > amtleft) ? amtleft : wbytes);
> > int offset = amtread - amtleft;
> >
> > if (is_dest_file && memcmp(buf+offset, zerobuf, interval) == 0) {
> > @@ -179,7 +194,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> > goto cleanup;
> >
> > }
> > - } while ((amtleft -= 512) > 0);
> > + } while ((amtleft -= interval) > 0);
> > }
> >
> > if (VIR_CLOSE(inputfd) < 0) {
>
>
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Minoru Usui <usui@xxxxxxxxxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list