[Libguestfs] [PATCH v2 1/2] lib: change how hbin sections are read.

[Richard W.M. Jones]

On Wed, Feb 15, 2017 at 05:22:52PM -0500, Dawid Zamirski wrote:
> On Wed, 2017-02-15 at 16:50 -0500, Dawid Zamirski wrote:
> > On Wed, 2017-02-15 at 21:14 +0000, Richard W.M. Jones wrote:
> > > On Wed, Feb 15, 2017 at 02:28:41PM -0500, Dawid Zamirski wrote:
> > > > Correct, however there's also no guarantee that seeking by 4k in
> > > > "garbage" data would not land you in registry data that happens
> > > > to
> > > > evaluate to "hbin" as well. That's why I put "hbin" offset
> > > > validation
> > > > check couple of lines below to make sure that the "hbin" we found
> > > > by
> > > > searching is a proper one. The offset check I'm referring to is:
> > > > 
> > > > /* get "stated" hbin offset from header */
> > > > size_t page_offset = le32to(page->offset_first) + 0x1000;
> > > > 
> > > > /* if that does not match our current file offset,
> > > >    then exit with error */
> > > > if (page_offset != off) { 
> > > >   SET_ERRNO...
> > > > }
> > > 
> > > Still, what kind of corruption would move a genuine hbin to a
> > > non-page-sized offset in the file?  It seems unlikely to me ...
> > > 
> > > Rich.
> > > 
> > 
> > None that I know of :-) However I don't see any other simple way to
> > silence that GCC7 warning and it seems harmless that way to me
> > (besides
> > the fact that the loop will have to make more iterations)
> 
> So I've found a way to keep offsetting by 4k and keep GCC7 happy:
> 
> while (off < h->endpages) {
>   if (off + 0x1000 > off)
>     off += 0x1000;
>   else
>     break; // off would overflow
> 
> Is this acceptable?

Yes, or even how about this (not tried it):

while (off <= h->endpages - 0x1000) {
  ...
}

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html