[CRIU] [PATCH] Fixed OVERFLOW_BEFORE_WIDEN issue introduced by remote images code.

[Rodrigo Bruno]

Hi,

2017-03-24 4:57 GMT+00:00 Andrei Vagin <avagin at virtuozzo.com>:

> On Sun, Mar 19, 2017 at 09:55:23PM +0000, rodrigo-bruno wrote:
> > Signed-off-by: rodrigo-bruno <rbruno at gsd.inesc-id.pt>
> > ---
> >  criu/img-remote-proto.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/criu/img-remote-proto.c b/criu/img-remote-proto.c
> > index 3751b6f..51584bb 100644
> > --- a/criu/img-remote-proto.c
> > +++ b/criu/img-remote-proto.c
> > @@ -743,7 +743,7 @@ int64_t send_image(int fd, struct rimage *rimg, int
> flags, bool close_fd)
> >                       } else if (rimg->curr_sent_bytes ==
> rimg->curr_sent_buf->nbytes) {
> >                               if (close_fd)
> >                                       close(fd);
> > -                            return nblocks*BUF_SIZE +
> rimg->curr_sent_buf->nbytes;
> > +                            return ((int64_t)nblocks*BUF_SIZE) +
> rimg->curr_sent_buf->nbytes;
>
> I'm not sure that I understand the logic of this function. For me it
> looks incorrect.
>
> The first thing is that I don't understand why you need to spit data to
> blocks.
>

Data is split into blocks because the image proxy does not know the size of
the image beforehand.
I.e., CRIU dump operation will open several images and will write directly
to the proxy (through a UNIX
socket). If we are able to know, upon image opening, how much data we CRIU
dump will need, them
we can avoid blocks.


>
> The second thing is that I think the following line is incorrect:
>
> min(BUF_SIZE, rimg->curr_sent_buf->nbytes) - rimg->curr_sent_bytes,
>
> nbytes is a constant in this function and it should be incorrect if
> nbytes is greater than BUF_SIZE and it isn't multiple to BUF_SIZE.
>
>
nbytes is the number of bytes that are being used in a block.
BUF_SIZE is the maximum number of used bytes in a block.
curr_sent_bytes is the number of bytes sent from the current block.

To send an image, we iterate through all blocks, sending one at a time.

min(BUF_SIZE, rimg->curr_sent_buf->nbytes) -> tells us how many bytes are
being used in the block.

min(BUF_SIZE, rimg->curr_sent_buf->nbytes) - rimg->curr_sent_bytes -> tells
us how many used bytes
still need to be sent.

Addressing your question: by design, nbytes should never be greater than
BUF_SIZE (unless
there is a bug...). Is should always be true that nbytes <= BUF_SIZE.

Nevertheless, I'm open to suggestions to improve the code/design of the
function :-)



> >                       }
> >               } else if (errno == EPIPE || errno == ECONNRESET) {
> >                       pr_warn("Connection for %s:%s was closed early
> than expected\n",
> > --
> > 2.1.4
> >
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20170324/b6364032/attachment-0001.html>